Re: [edk2] Creating my own flashing app

[Andrew Fish]

On a secure platform you likely need to update using a secure capsule. 
https://github.com/tianocore/tianocore.github.io/wiki/Capsule-Based-Firmware-Update-and-Firmware-Recovery
 
The capsule is the standard method, and then all the FLASH update code is part 
of the ROM.

Generally since an EFI platform has NVRAM services in the NOR FLASH there is an 
SPI driver to write to FLASH.

So if your platform does not secure FLASH you can use the services from the ROM.

Sent from my iPhone

> On Dec 3, 2018, at 8:45 PM, Guy Raviv  wrote:
> 
>   a whole SPI BIOS image.
> if i was not clear please tell me what i'm missing.
> 
> Thanks!
> Guy
> 
>   Virus-free. www.avg.com
> 
>> On Tue, Dec 4, 2018 at 6:42 AM Andrew Fish  wrote:
>> Guy,
>> 
>> What are you trying to FLASH?
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>> > On Dec 3, 2018, at 7:28 PM, Guy Raviv  wrote:
>> > 
>> > Hi,
>> > 
>> > I want to create my own flashing utility.
>> > Is there any EDKII App/Utilities that can help me?
>> > 
>> > Thanks,
>> > Guy
>> > 
>> > 
>> > Virus-free.
>> > www.avg.com
>> > 
>> > <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>> > ___
>> > 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] Creating my own flashing app

[Kevin D Davis]

  
  


Ok, so what is going wrong?  Have you looked at any of the flash tools 
in other open source projects?


Kevin  

  



On Mon, Dec 3, 2018 at 10:45 PM -0600, "Guy Raviv"  wrote:










  a whole SPI BIOS image.
if i was not clear please tell me what i'm missing.

Thanks!
Guy


Virus-free.
www.avg.com

<#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

On Tue, Dec 4, 2018 at 6:42 AM Andrew Fish  wrote:

> Guy,
>
> What are you trying to FLASH?
>
> Thanks,
>
> Andrew Fish
>
> > On Dec 3, 2018, at 7:28 PM, Guy Raviv  wrote:
> >
> > Hi,
> >
> > I want to create my own flashing utility.
> > Is there any EDKII App/Utilities that can help me?
> >
> > Thanks,
> > Guy
> >
> > <
> http://www.avg.com/email-signature?utm_medium=email_source=link_campaign=sig-email_content=webmail
> >
> > Virus-free.
> > www.avg.com
> > <
> http://www.avg.com/email-signature?utm_medium=email_source=link_campaign=sig-email_content=webmail
> >
> > <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
> > ___
> > 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] Creating my own flashing app

[Guy Raviv]

  a whole SPI BIOS image.
if i was not clear please tell me what i'm missing.

Thanks!
Guy


Virus-free.
www.avg.com

<#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

On Tue, Dec 4, 2018 at 6:42 AM Andrew Fish  wrote:

> Guy,
>
> What are you trying to FLASH?
>
> Thanks,
>
> Andrew Fish
>
> > On Dec 3, 2018, at 7:28 PM, Guy Raviv  wrote:
> >
> > Hi,
> >
> > I want to create my own flashing utility.
> > Is there any EDKII App/Utilities that can help me?
> >
> > Thanks,
> > Guy
> >
> > <
> http://www.avg.com/email-signature?utm_medium=email_source=link_campaign=sig-email_content=webmail
> >
> > Virus-free.
> > www.avg.com
> > <
> http://www.avg.com/email-signature?utm_medium=email_source=link_campaign=sig-email_content=webmail
> >
> > <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
> > ___
> > 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] Creating my own flashing app

[Andrew Fish]

Guy,

What are you trying to FLASH?

Thanks,

Andrew Fish

> On Dec 3, 2018, at 7:28 PM, Guy Raviv  wrote:
> 
> Hi,
> 
> I want to create my own flashing utility.
> Is there any EDKII App/Utilities that can help me?
> 
> Thanks,
> Guy
> 
> 
> Virus-free.
> www.avg.com
> 
> <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
> ___
> 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] EmbeddedPkg : Corrected flow for setting Buswidth for eMMC

[Meenakshi Aggarwal]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
index f661a0c..9118eb2 100755
--- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
+++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
@@ -309,11 +309,14 @@ InitializeEmmcDevice (
   }
   Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, BusMode);
   if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "InitializeEmmcDevice(): Failed to set EXTCSD bus 
width, Status:%r\n", Status));
+continue;
+  } else {
+DEBUG ((DEBUG_INFO, "InitializeEmmcDevice(): Set EXTCSD bus width %d 
successfully\n", BusMode));
+break;
   }
-  return Status;
 }
   }
+
   return Status;
 }
 
-- 
1.9.1

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

[edk2] Creating my own flashing app

[Guy Raviv]

Hi,

I want to create my own flashing utility.
Is there any EDKII App/Utilities that can help me?

Thanks,
Guy


Virus-free.
www.avg.com

<#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms 14/27] Silicon/NXP: Add i.MX6 GPT and EPIT timer headers

[Chris Co via edk2-devel]

Hi Leif,

> -Original Message-
> From: Leif Lindholm 
> Sent: Thursday, November 8, 2018 10:14 AM
> To: Chris Co 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Michael D Kinney 
> Subject: Re: [PATCH edk2-platforms 14/27] Silicon/NXP: Add i.MX6 GPT and
> EPIT timer headers
> 
> On Fri, Sep 21, 2018 at 08:26:05AM +, Chris Co wrote:
> > This adds the definitions for the NXP i.MX6 General Purpose Timer and
> > the Enhanced Periodic Interrupt Timer modules.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Christopher Co 
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Cc: Michael D Kinney 
> > ---
> >  Silicon/NXP/iMX6Pkg/Include/common_epit.h | 118 +
> > Silicon/NXP/iMX6Pkg/Include/common_gpt.h  | 271
> 
> >  2 files changed, 389 insertions(+)
> >
> > diff --git a/Silicon/NXP/iMX6Pkg/Include/common_epit.h
> > b/Silicon/NXP/iMX6Pkg/Include/common_epit.h
> > new file mode 100644
> > index ..485d6ccbc51e
> > --- /dev/null
> > +++ b/Silicon/NXP/iMX6Pkg/Include/common_epit.h
> 
> Rename to CamelCase?
> 

Can do. Would you prefer the iMX prefix to be in CamelCase too? (i.e. 
Imx6Epit.h)

> > @@ -0,0 +1,118 @@
> > +/** @file
> > +*
> > +*  Provides definitions for the EPIT (Enhanced Periodic Interrupt
> > +Timer)
> > +*  module that are common to Freescale SoCs.
> > +*
> > +*  Copyright (c) 2018 Microsoft Corporation. All rights reserved.
> > +*  Copyright (c) 2004-2010, Freescale Semiconductor, Inc. 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
> > +*
> > +https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopens
> > +ource.org%2Flicenses%2Fbsd-
> license.phpdata=02%7C01%7CChristopher
> >
> +.Co%40microsoft.com%7C5da90dbbf4b5452a4c8e08d645a605cc%7C72f988
> bf86f1
> >
> +41af91ab2d7cd011db47%7C1%7C0%7C636772976703090578sdata=
> g3OOywqu7
> > +VDuq39F3PVs8SaXZVbA7h77F7riMUXb0lE%3Dreserved=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 __COMMON_EPIT_H
> > +#define __COMMON_EPIT_H
> 
> COMMON is a bit too common. Can we at least stick an IMX on the front?
> 
> This is really also a problem for all of the structs and #defines below as 
> well.
> 
> > +
> > +typedef struct {
> > +  UINT32 CR;
> > +  UINT32 SR;
> > +  UINT32 LR;
> > +  UINT32 CMPR;
> > +  UINT32 CNT;
> > +} CSP_EPIT_REG, *PCSP_EPIT_REG;
> 
> I'm really not a fan of typedef pointers. Please drop the *PSCP one.
> 
> > +
> > +#define EPIT_CR_OFFSET  0x
> > +#define EPIT_SR_OFFSET  0x0004
> > +#define EPIT_LR_OFFSET  0x0008
> > +#define EPIT_CMPR_OFFSET0x000C
> > +#define EPIT_CNR_OFFSET 0x0010
> > +
> > +#define EPIT_CR_EN_LSH  0
> 
> What is LSH?
> 
> > +#define EPIT_CR_ENMOD_LSH   1
> > +#define EPIT_CR_OCIEN_LSH   2
> > +#define EPIT_CR_RLD_LSH 3
> > +#define EPIT_CR_PRESCALAR_LSH   4
> > +#define EPIT_CR_SWR_LSH 16
> > +#define EPIT_CR_IOVW_LSH17
> > +#define EPIT_CR_DBGEN_LSH   18
> > +#define EPIT_CR_WAITEN_LSH  19
> > +#define EPIT_CR_DOZEN_LSH   20
> > +#define EPIT_CR_STOPEN_LSH  21
> > +#define EPIT_CR_OM_LSH  22
> > +#define EPIT_CR_CLKSRC_LSH  24
> > +
> > +#define EPIT_SR_OCIF_LSH0
> > +#define EPIT_LR_LOAD_LSH0
> > +#define EPIT_CMPR_COMPARE_LSH   0
> > +#define EPIT_CNT_COUNT_LSH  0
> > +
> > +#define EPIT_CR_EN_WID  1
> 
> What is WID?
> 

LSH is the number of bits to left shift to get to the specified bitfield in the 
register. WID is the length of the specified bitfield.
I don't like this register access pattern at all. Will define structs of the 
registers instead...

Thanks,
Chris

> > +#define EPIT_CR_ENMOD_WID   1
> > +#define EPIT_CR_OCIEN_WID   2
> > +#define EPIT_CR_RLD_WID 1
> > +#define EPIT_CR_PRESCALAR_WID   12
> > +#define EPIT_CR_SWR_WID 1
> > +#define EPIT_CR_IOVW_WID1
> > +#define EPIT_CR_DBGEN_WID   1
> > +#define EPIT_CR_WAITEN_WID  1
> > +#define EPIT_CR_DOZEN_WID   1
> > +#define EPIT_CR_STOPEN_WID  1
> > +#define EPIT_CR_OM_WID  2
> > +#define EPIT_CR_CLKSRC_WID  2
> > +
> > +#define EPIT_SR_OCIF_WID1
> > +#define EPIT_LR_LOAD_WID32
> > +#define EPIT_CMPR_COMPARE_WID   32
> > +#define EPIT_CNT_COUNT_WID  32
> > +
> > +// CR
> > +#define EPIT_CR_EN_DISABLE  0
> > +#define EPIT_CR_EN_ENABLE   1
> > +
> > +#define EPIT_CR_ENMOD_RESUME0
> > +#define EPIT_CR_ENMOD_LOAD  1
> > +
> > +#define EPIT_CR_OCIEN_DISABLE   0
> > +#define EPIT_CR_OCIEN_ENABLE1
> > +
> > +#define 

Re: [edk2] [Patch v1 1/1] BaseTools: create and use a standard shared variable for '*'

[Zhu, Yonghong]

Yes.  I prefer not to change it.

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Monday, December 03, 2018 11:19 PM
To: Zhu, Yonghong ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: RE: [Patch v1 1/1] BaseTools: create and use a standard shared 
variable for '*'

I was trying to change all use of *, without regard to the usage of it.

Do you think that mathematical * should not be changed?

-Jaben

> -Original Message-
> From: Zhu, Yonghong
> Sent: Sunday, December 02, 2018 6:31 PM
> To: Carsey, Jaben ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zhu, Yonghong 
> 
> Subject: RE: [Patch v1 1/1] BaseTools: create and use a standard 
> shared variable for '*'
> Importance: High
> 
> Hi Jaben,
> 
> In this patch, it also changed the mathematics multiplicative '*' 
> (used in
> expression) to  TAB_STAR, is it by on purpose?
> Eg:
> -NonLetterOpLst = ['+', '-', '*', '/', '%', '&', '|', '^', '~', '<<', 
> '>>', '!', '=', '>', '<',
> '?', ':']
> +NonLetterOpLst = ['+', '-', TAB_STAR, '/', '%', '&', '|', '^', 
> + '~', '<<', '>>', '!', '=',
> '>', '<', '?', ':']
> 
> Best Regards,
> Zhu Yonghong
> 
> -Original Message-
> From: Carsey, Jaben
> Sent: Friday, November 16, 2018 11:40 PM
> To: edk2-devel@lists.01.org
> Cc: Zhu, Yonghong ; Gao, Liming 
> 
> Subject: [Patch v1 1/1] BaseTools: create and use a standard shared 
> variable for '*'
> 
> add a variable for the string '*' and then use it instead of lots of '*'
> 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jaben Carsey 
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 54 ++---
> ---
>  BaseTools/Source/Python/AutoGen/BuildEngine.py | 10 ++--
>  BaseTools/Source/Python/BPDG/GenVpd.py | 14 ++---
>  BaseTools/Source/Python/Common/DataType.py |  1 +
>  BaseTools/Source/Python/Common/Expression.py   |  4 +-
>  BaseTools/Source/Python/Common/Misc.py |  2 +-
>  BaseTools/Source/Python/Common/ToolDefClassObject.py   | 23 +
>  BaseTools/Source/Python/Common/VpdInfoFile.py  |  8 +--
>  BaseTools/Source/Python/GenFds/FdfParser.py|  5 +-
>  BaseTools/Source/Python/GenFds/GenFds.py   |  2 +-
>  BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py |  8 +--
>  BaseTools/Source/Python/GenFds/Section.py  |  2 +-
>  BaseTools/Source/Python/Workspace/DscBuildData.py  |  6 +--
>  13 files changed, 70 insertions(+), 69 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index f3560bfc787d..25417c447061 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -1438,7 +1438,7 @@ class PlatformAutoGen(AutoGen):
>  PcdValue = Sku.DefaultValue
>  if PcdValue == "":
>  PcdValue  = Pcd.DefaultValue
> -if Sku.VpdOffset != '*':
> +if Sku.VpdOffset != TAB_STAR:
>  if PcdValue.startswith("{"):
>  Alignment = 8
>  elif PcdValue.startswith("L"):
> @@ -1462,7 +1462,7 @@ class PlatformAutoGen(AutoGen):
>  VpdFile.Add(Pcd, SkuName, Sku.VpdOffset)
>  SkuValueMap[PcdValue].append(Sku)
>  # if the offset of a VPD is *, then it need 
> to be fixed up by third party tool.
> -if not NeedProcessVpdMapFile and Sku.VpdOffset == 
> "*":
> +if not NeedProcessVpdMapFile and 
> + Sku.VpdOffset ==
> TAB_STAR:
>  NeedProcessVpdMapFile = True
>  if self.Platform.VpdToolGuid is None or 
> self.Platform.VpdToolGuid == '':
>  EdkLogger.error("Build", 
> FILE_NOT_FOUND, \ @@ -1522,7
> +1522,7 @@ class PlatformAutoGen(AutoGen):
>  PcdValue = Sku.DefaultValue
>  if PcdValue == "":
>  PcdValue  = DscPcdEntry.DefaultValue
> -if Sku.VpdOffset != '*':
> +if Sku.VpdOffset != TAB_STAR:
>  if PcdValue.startswith("{"):
>  Alignment = 8
>  elif PcdValue.startswith("L"):
> @@ -1545,7 +1545,7 @@ class PlatformAutoGen(AutoGen):
>  SkuValueMap[PcdValue] = []
>  VpdFile.Add(DscPcdEntry, SkuName, 
> Sku.VpdOffset)
>  SkuValueMap[PcdValue].append(Sku)
> -if not NeedProcessVpdMapFile and 
> Sku.VpdOffset == "*":
> 

Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

[Chris Co via edk2-devel]

> -Original Message-
> From: Leif Lindholm 
> Sent: Monday, December 3, 2018 1:43 AM
> To: Chris Co 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Michael D Kinney 
> Subject: Re: [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-
> specific i.MX packages to use
> 
> On Sat, Dec 01, 2018 at 12:22:17AM +, Chris Co wrote:
> > > If using EFI_ACPI prefix, these #defines really should be in edk2
> > > MdePkg. And CSRT itself is, so that might not be a bad idea.
> > >
> > > > +
> > > > +#pragma pack(push, 1)
> > >
> > > I don't see this #pragma making any difference to the structs below,
> > > can it be dropped?
> >
> > The pragma pack is defensive. Without it, we rely on the compiler
> > packing structures by default and this may not happen on 64 bit
> > compiles.
> 
> I understand that is what the pragma does. My comment was because all of the
> variables in the below structs are naturally aligned.
> The reason I dislike its use when effectively a no-op, is that it makes it 
> look like it
> it isn't a no-op.
> 
> If it covers a larger set of structs, some of which require the packed 
> attribute I'm
> OK with that. But I'm not a fan of adding it "just in case" without 
> contemplating
> the statement's (lack of) effect.
> 
> Regards,
> 
> Leif
> 

Makes sense. I am checking to make sure this pragma wasn't included due to some 
observed compiler behavior on our end, since this header is also used on our 
ARM64 work.
Will remove it once confirmed it is safe.

Thanks,
Chris

> > I have addressed the remaining feedback and will resubmit with v2.
> >
> > Thanks,
> > Chris
> >
> > > > +//---
> > > > +
> > > > +- // CSRT Resource Group header 24 bytes long
> > > > +//---
> > > > +
> > > > +-
> > > > +typedef struct {
> > > > +  UINT32 Length;
> > > > +  UINT32 VendorID;
> > > > +  UINT32 SubVendorId;
> > > > +  UINT16 DeviceId;
> > > > +  UINT16 SubdeviceId;
> > > > +  UINT16 Revision;
> > > > +  UINT16 Reserved;
> > > > +  UINT32 SharedInfoLength;
> > > > +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> > > > +
> > > > +//---
> > > > +
> > > > +- // CSRT Resource Descriptor 12 bytes total
> > > > +//---
> > > > +
> > > > +-
> > > > +typedef struct {
> > > > +  UINT32 Length;
> > > > +  UINT16 ResourceType;
> > > > +  UINT16 ResourceSubType;
> > > > +  UINT32 UID;
> > > > +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
> > > > +#pragma pack (pop)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms 12/27] Silicon/NXP: Add i.MX6 I/O MUX library

[Chris Co via edk2-devel]

Hi Leif,

> -Original Message-
> From: Leif Lindholm 
> Sent: Thursday, November 8, 2018 10:00 AM
> To: Chris Co 
> Cc: edk2-devel@lists.01.org; Ard Biesheuvel ;
> Michael D Kinney 
> Subject: Re: [PATCH edk2-platforms 12/27] Silicon/NXP: Add i.MX6 I/O MUX
> library
> 
> On Fri, Sep 21, 2018 at 08:26:03AM +, Chris Co wrote:
> > This adds support for initializing and manipulating the I/O Pads on
> > NXP i.MX6 SoCs.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Christopher Co 
> > Cc: Ard Biesheuvel 
> > Cc: Leif Lindholm 
> > Cc: Michael D Kinney 
> > ---
> >  Silicon/NXP/iMX6Pkg/Library/iMX6IoMuxLib/iMX6IoMux.c  | 151
> 
> >  Silicon/NXP/iMX6Pkg/Library/iMX6IoMuxLib/iMX6IoMuxLib.inf |  41
> > ++
> >  2 files changed, 192 insertions(+)
> >
> > diff --git a/Silicon/NXP/iMX6Pkg/Library/iMX6IoMuxLib/iMX6IoMux.c
> > b/Silicon/NXP/iMX6Pkg/Library/iMX6IoMuxLib/iMX6IoMux.c
> > new file mode 100644
> > index ..7c0c7b54a2fe
> > --- /dev/null
> > +++ b/Silicon/NXP/iMX6Pkg/Library/iMX6IoMuxLib/iMX6IoMux.c
> > @@ -0,0 +1,151 @@
> > +/** @file
> > +*
> > +*  Copyright (c) 2018 Microsoft Corporation. All rights reserved.
> > +*
> > +*  This program and the accompanying materials
> > +*  are licensed and made available under the terms and conditions of
> > +the BSD License
> > +*  which accompanies this distribution.  The full text of the license
> > +may be found at
> > +*
> > +https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fopens
> > +ource.org%2Flicenses%2Fbsd-
> license.phpdata=02%7C01%7CChristopher
> >
> +.Co%40microsoft.com%7C416f86ef76c04f1cac6408d645a40e53%7C72f988b
> f86f1
> >
> +41af91ab2d7cd011db47%7C1%7C0%7C636772968261577670sdata=
> vkaVmLhCg
> > +d0X80xpryCJ4kHPUTUtcv6W6MFg3a082nk%3Dreserved=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.
> > +*
> > +**/
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +// Muxing functions
> > +VOID
> > +ImxPadConfig (
> > +  IN  IMX_PAD Pad,
> > +  IN  IMX_PADCFG  PadConfig
> 
> I'll get back to reviewing patch 11 at some point, but that one is hard 
> going. So
> I'll point out here what I'll mention then:
> Please just use UINT64. Typedefs are useful to reduce pointless repetition for
> structs, but here it just obscures what is programatically going on.
> 

I'm thinking to rework the PADCFG to actually use structs properly instead of 
custom-packing with macros. It seems the previous dev was very macro happy and 
I am the opposite...
The remaining feedback makes sense. Will address for v2.

Thanks,
Chris

> > +  )
> > +{
> > +  // Configure Mux Control
> > +  MmioWrite32 (
> > +IMX_IOMUXC_BASE + _IMX_PAD_MUX_OFFSET (Pad),
> > +_IMX_PADCFG_MUX_CTL (PadConfig));
> 
> It would be worth adding macros or simple accessor functions for these - 
> there's
> a lot of text in this file that has no semantic value and needs to be manually
> filtered when reading.
> 
> I.e. IomuxWrite32 (Pad, ...), IomuxRead32 (Pad), ...
> 
> Other comment really belonging to 11:
> Please drop leading _ from macros. Such macros are intended for internal use
> by toolchains.
> 
> > +
> > +  // Configure Select Input Control
> > +  if (_IMX_PADCFG_SEL_INP (PadConfig) != 0) {
> > +DEBUG ((DEBUG_INFO, "Setting INPUT_SELECT %x value %x\n",
> > +_IMX_SEL_INP_REGISTER (_IMX_PADCFG_SEL_INP (PadConfig)),
> > +_IMX_SEL_INP_VALUE (_IMX_PADCFG_SEL_INP (PadConfig;
> > +
> > +MmioWrite32 (
> > +  _IMX_SEL_INP_REGISTER (_IMX_PADCFG_SEL_INP (PadConfig)),
> > +  _IMX_SEL_INP_VALUE (_IMX_PADCFG_SEL_INP (PadConfig)));  }
> > +
> > +  // Configure Pad Control
> > +  MmioWrite32 (
> > +IMX_IOMUXC_BASE + _IMX_PAD_CTL_OFFSET (Pad),
> > +_IMX_PADCFG_PAD_CTL (PadConfig)); }
> > +
> > +VOID
> > +ImxPadDumpConfig (
> > +  IN  CHAR8 *SignalFriendlyName,
> > +  IN  IMX_PAD Pad
> > +  )
> > +{
> > +  IMX_IOMUXC_MUX_CTL  MuxCtl;
> > +  IMX_IOMUXC_PAD_CTL  PadCtl;
> > +
> > +  MuxCtl.AsUint32 = MmioRead32 (
> > +  IMX_IOMUXC_BASE + _IMX_PAD_MUX_OFFSET (Pad));
> > +
> > +  DEBUG ((
> > +   DEBUG_INIT,
> > +   "- %a MUX_CTL(0x%p)=0x%08x: MUX_MODE:%d SION:%d | ",
> > +   SignalFriendlyName,
> > +   IMX_IOMUXC_BASE + _IMX_PAD_MUX_OFFSET (Pad),
> > +   MuxCtl.AsUint32,
> > +   MuxCtl.Fields.MUX_MODE,
> > +   MuxCtl.Fields.SION));
> > +
> > +  PadCtl.AsUint32 = MmioRead32 (
> > +  IMX_IOMUXC_BASE + _IMX_PAD_CTL_OFFSET (Pad));
> > +
> > +  DEBUG ((
> > +   DEBUG_INIT,
> > +   "PAD_CTL(0x%p)=0x%08x: SRE:%d DSE:%d SPEED:%d ODE:%d PKE:%d
> PUE:%d PUS:%d HYS:%d\n",
> > +   IMX_IOMUXC_BASE + _IMX_PAD_CTL_OFFSET (Pad),
> > +   PadCtl.AsUint32,
> > +   

Re: [edk2] [PATCH v4 0/2] Remove DuetPkg and unused tools

[Wu, Hao A]

> -Original Message-
> From: Zhang, Shenglei
> Sent: Monday, December 03, 2018 10:18 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Wu, Hao A; Zhu, Yonghong; Gao, Liming
> Subject: [PATCH v4 0/2] Remove DuetPkg and unused tools
> 
> DuetPkg depends on Legacy BIOS to provide a UEFI environment.
> It was invented in the era when UEFI environment is hard to find.
> Since now UEFI is very popular in PC area, we could stop the
> official support of this package and remove it from the master.
> And moreover, the tools only used by DuetPkg can also be removed.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1322
> 
> The changes are placed in
> https://github.com/shenglei10/edk2/commits/duet
> 
> v2:Remove these tools in Makefile and GNUmakefile.
> 
> v3:Change the commit order.
> 
> v4:Remove these tools in BinWrappers/PosixLike/ and
>UserManuals.

The BaseTools part change seems good to me.
Reviewed-by: Hao Wu 

Best Regards,
Hao Wu

> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Shenglei Zhang (2):
>   DuetPkg: Remove DuetPkg
>   BaseTools: Remove tools only used by DuetPkg
> 
>  BaseTools/Source/BinaryFiles.txt  |4 -
>  BaseTools/Source/C/BootSectImage/GNUmakefile  |   21 -
>  BaseTools/Source/C/BootSectImage/Makefile |   22 -
>  .../Source/C/BootSectImage/bootsectimage.c|  955 --
>  BaseTools/Source/C/BootSectImage/fat.h|  152 -
>  BaseTools/Source/C/BootSectImage/mbr.h|   58 -
>  BaseTools/Source/C/EfiLdrImage/EfiLdrImage.c  |  319 --
>  BaseTools/Source/C/EfiLdrImage/GNUmakefile|   21 -
>  BaseTools/Source/C/EfiLdrImage/Makefile   |   22 -
>  BaseTools/Source/C/GNUmakefile|5 -
>  BaseTools/Source/C/GenBootSector/FatFormat.h  |  152 -
>  .../Source/C/GenBootSector/GenBootSector.c|  823 -
>  .../Source/C/GenBootSector/GetDrvNumOffset.c  |   73 -
>  BaseTools/Source/C/GenBootSector/Makefile |   22 -
>  BaseTools/Source/C/GenPage/GNUmakefile|   21 -
>  BaseTools/Source/C/GenPage/GenPage.c  |  441 ---
>  BaseTools/Source/C/GenPage/Makefile   |   22 -
>  BaseTools/Source/C/GenPage/VirtualMemory.h|  122 -
>  .../Source/C/GnuGenBootSector/FatFormat.h |  152 -
>  .../Source/C/GnuGenBootSector/GNUmakefile |   21 -
>  .../C/GnuGenBootSector/GnuGenBootSector.c |  455 ---
>  BaseTools/Source/C/Makefile   |4 -
>  BaseTools/toolsetup.bat   |4 -
>  DuetPkg/AcpiResetDxe/Reset.c  |  212 --
>  DuetPkg/AcpiResetDxe/Reset.inf|   47 -
>  DuetPkg/BiosVideoThunkDxe/BiosVideo.c | 2822 -
>  DuetPkg/BiosVideoThunkDxe/BiosVideo.h |  504 ---
>  DuetPkg/BiosVideoThunkDxe/BiosVideo.inf   |   50 -
>  DuetPkg/BiosVideoThunkDxe/ComponentName.c |  166 -
>  DuetPkg/BiosVideoThunkDxe/LegacyBiosThunk.c   |  220 --
>  .../BiosVideoThunkDxe/VesaBiosExtensions.h|  457 ---
>  DuetPkg/BootSector/BootSector.inf |   79 -
>  DuetPkg/BootSector/FILE.LST   |   39 -
>  DuetPkg/BootSector/GNUmakefile|  140 -
>  DuetPkg/BootSector/Gpt.S  |  297 --
>  DuetPkg/BootSector/Gpt.asm|  294 --
>  DuetPkg/BootSector/Makefile   |  173 -
>  DuetPkg/BootSector/Mbr.S  |  262 --
>  DuetPkg/BootSector/Mbr.asm|  261 --
>  DuetPkg/BootSector/bin/Gpt.com|  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/Mbr.com|  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/Readme.txt |8 -
>  DuetPkg/BootSector/bin/St16_64.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/St32_64.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start.com  |  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start16.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start32.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/Start64.com|  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/bootsect.com   |  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/bs16.com   |  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/bs32.com   |  Bin 512 -> 0 bytes
>  DuetPkg/BootSector/bin/efi32.com  |  Bin 139264 -> 0 bytes
>  DuetPkg/BootSector/bin/efi32.com2 |  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bin/efi64.com  |  Bin 139264 -> 0 bytes
>  DuetPkg/BootSector/bin/efi64.com2 |  Bin 4096 -> 0 bytes
>  DuetPkg/BootSector/bootsect.S |  303 --
>  DuetPkg/BootSector/bootsect.asm   |  301 --
>  DuetPkg/BootSector/bs16.S |  291 --
>  DuetPkg/BootSector/bs16.asm   |  288 --
>  DuetPkg/BootSector/bs32.S |  312 --
>  DuetPkg/BootSector/bs32.asm   |  310 --
>  DuetPkg/BootSector/efi32.S| 1176 ---
>  

Re: [edk2] [PATCH] MdePkg-BaseLib: PathCleanUpDirectories fix

[Jim.Dailey]

Liming and Mike,

I don't feel there is any logical issue with this proposed fix.

However, there is some shell code that fails when the fix is in place.
I think that shell code probably should be changed; I'll see if I can
make an acceptable change there first.

Please disregard this patch; I'll likely resubmit some version of it
later on.

Regards,
Jim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dailey, 
Jim
Sent: Friday, November 30, 2018 9:43 AM
To: liming@intel.com; michael.d.kin...@intel.com
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdePkg-BaseLib: PathCleanUpDirectories fix


[EXTERNAL EMAIL] 

Oops!  I think this change may have an issue.

Hold off and I'll let you know if that's the case.

--Jim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Dailey, 
Jim
Sent: Friday, November 30, 2018 9:12 AM
To: liming@intel.com; michael.d.kin...@intel.com
Cc: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdePkg-BaseLib: PathCleanUpDirectories fix


PathCleanUpDirectories does not handle "\..\" properly; it
returns "\" instead of "".  This change fixes that
problem so that "" is returned.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jim Dailey 
---
 MdePkg/Library/BaseLib/FilePaths.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseLib/FilePaths.c 
b/MdePkg/Library/BaseLib/FilePaths.c
index 92e4c350ff..69e46dd135 100644
--- a/MdePkg/Library/BaseLib/FilePaths.c
+++ b/MdePkg/Library/BaseLib/FilePaths.c
@@ -110,7 +110,12 @@ PathCleanUpDirectories(
  ((*(TempString + 3) == L'\\') || (*(TempString + 3) == CHAR_NULL))
 ) {
 *(TempString + 1) = CHAR_NULL;
-PathRemoveLastItem(Path);
+if (!PathRemoveLastItem(Path)) {
+  //
+  // We had "\.."
+  //
+  *Path = CHAR_NULL;
+}
 if (*(TempString + 3) != CHAR_NULL) {
   CopyMem (Path + StrLen (Path), TempString + 4, StrSize (TempString + 4));
 }
-- 
2.17.0.windows.1

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

Re: [edk2] [edk2-announce] Research Request

[Rebecca Cran via edk2-devel]

On Monday, 3 December 2018 02:29:28 MST Laszlo Ersek wrote:
> On 11/29/18 22:20, Rebecca Cran wrote:
> > Would you be interested in going through this process with Phabricator,
> > too?
> Sure! Just tell me where to create an account.

Go to https://code.bluestop.org/auth/register/ to create a new account on the 
system, or https://code.bluestop.org/auth/ to login using an existing GitHub 
account.

- -
Rebecca


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

Re: [edk2] SCT bugzilla topic?

[Supreeth Venkatesh]

Leif,

Earlier, we used to use UTWG Mantis (for feature requests - 
https://mantis.uefi.org/mantis/view.php).
After, UEFI-SCT became Open source, we did discuss to have the same 
infrastructure setup as edk2, which means using Bugzilla.
However, I don't think we have created a SCT component or Edk2 Test Product yet.

Your email reminds me one more task after the SCT component / Edk2 Test product 
is created i.e.,
to migrate over remaining UTWG issues to Bugzilla.

Once Eric chimes in/agrees, we will request Mike to create Edk2 Test Product 
within Bugzilla.

Thanks,
Supreeth

-Original Message-
From: Leif Lindholm 
Sent: Monday, December 3, 2018 8:47 AM
To: edk2-devel@lists.01.org
Cc: Eric Jin ; Supreeth Venkatesh 
; Michael D Kinney 
Subject: SCT bugzilla topic?

Hi Eric, Supreeth, Mike,

I was looking to raise a feature request on UEFI SCT and didn't spot such a 
product. Should we create one?

Or perhaps we should have an EDK2 Test product with an SCT component?

Regards,

Leif
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] SCT bugzilla topic?

[Richardson, Brian]

I think "EDK2 Test product with an SCT component" is the best option.

Thanks ... br
---
Brian Richardson -- Director, Firmware Ecosystem Development
brian.richard...@intel.com -- @intel_brian (Twitter & WeChat)
https://software.intel.com/en-us/meet-the-developers/evangelists/team/brian-richardson
 

-Original Message-
From: edk2-devel  On Behalf Of Leif Lindholm
Sent: Monday, December 3, 2018 9:47 AM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Jin, Eric 

Subject: [edk2] SCT bugzilla topic?

Hi Eric, Supreeth, Mike,

I was looking to raise a feature request on UEFI SCT and didn't spot such a 
product. Should we create one?

Or perhaps we should have an EDK2 Test product with an SCT component?

Regards,

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

Re: [edk2] [edk2-announce] Research Request

[Jeremiah Cox via edk2-devel]

Laszlo,

Did you want to summarize your experience of our GitHub experiments?  From your 
comments on the PRs, it sounded like the email notifications did not provide 
the level of detail that you desire for archival purposes.  Stephano’s email 
suggested that as long as we have an alternative mechanism to archive all 
metadata, that may still be acceptable.  I propose that 
https://github.com/josegonzalez/python-github-backup may suffice.



Thank you,

Jeremiah




From: Laszlo Ersek 
Sent: Thursday, November 29, 2018 1:48:18 AM
To: Jeremiah Cox; Brian J. Johnson; stephano
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [edk2-announce] Research Request

On 11/29/18 02:07, Jeremiah Cox wrote:
> I did a further experiment for you:
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flersek%2Fedk2%2Fpull%2F2data=02%7C01%7Cjerecox%40microsoft.com%7Cab0bcfaae8d14af6b1ba08d655dfcc78%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636790817039792972sdata=ibzR7PEq%2FuT%2B4Q7ZTtDYow6sYKg%2B4Awj3cpFLD9vWhw%3Dreserved=0

Thanks!

> I cannot rebase away my history from PRs...
> Hopefully you have a nice email trail too.

The emails are coming in nice, but I'm not universally pleased with the
contents. I listed some issues regarding that in
,
 but I guess I should write them
up sometime more readably, at the end of this experiment.

Thanks!
Laszlo

> -Original Message-
> From: Laszlo Ersek 
> Sent: Wednesday, November 28, 2018 2:02 PM
> To: Jeremiah Cox ; Brian J. Johnson 
> ; stephano 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [edk2-announce] Research Request
>
> On 11/28/18 19:31, Jeremiah Cox wrote:
>> Test PR submitted
>
> Thanks!
> Laszlo
>

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

Re: [edk2] [RFC] Proposal to add edk2-apps repository

[Laszlo Ersek]

On 12/03/18 16:07, Leif Lindholm wrote:
> On Mon, Dec 03, 2018 at 03:11:33PM +0100, Laszlo Ersek wrote:
>> On 11/30/18 07:03, Andrew Fish wrote:
>>> Mike,
>>>
>>> As Krishna points out there are flavors of Apps. Do we want to have
>>> different packages for different flavor of apps, or different dirs in
>>> a more generic App package? Maybe we should define classes of UEFI
>>> Applications in the README.md and give them a place to live.
>>
>> In my opinion, this is absolutely the first step that should be done.
> 
> Yes please.
> 
>> Personally, I've just learned, from this thread, that there are *three*
>> (not two) UEFI application entry point types that edk2 supports.
>>
>> * I've always known about main() -- libc app --, and ShellAppMain() --
>>   shell app. I've always known these because I read about them in
>>   "AppPkg/ReadMe.txt" and "StdLib/ReadMe.txt" years ago. In particular,
>>   compare the description of "Hello" and "Main".
>>
>> * And now MdeModulePkg/Application has been mentioned, in this thread,
>>   where I see UefiMain() as the entry point.
> 
> Well, these are defined by the application .inf though, so "UefiMain"
> is just a common name picked. MdeModulePkg/Application also has
> applications with entry points called 'BootManagerMenuEntry',
> 'SmiHandlerProfileInfoEntrypoint' and 'InitializeUserInterface'. (The
> latter being UiApp.)

Right, I guess I should have referred to the "UefiApplicationEntryPoint"
lib class instead. (I did so below.)

> 
>> I don't recall reading about UefiMain() or UefiApplicationEntryPoint on
>> this list. On the other hand, I remember several discussions where
>> people asked if they could write an application and invoke it from
>> SysPrep or similar, and the answer has always been, "oh sorry you
>> can't do that, because the lowest level you can go is ShellAppMain(),
>> and that won't work from SysPrep".
> 
> Huh? Who claimed that? Where?

Sorry, don't have any specifics, just a lingering (but strong) memory.

> (I suspect the meaning of "Application" has taken on some
> overly-specific meaning for someone if they have claimed such - going
> back to how having a proper definition is clearly quite important.)
> 
> Of course they can. Any EDK2 image itself contains a bunch of .efi
> executables - and there is nothing preventing you from invoking those
> from the shell.

The dependency was the other way around. UEFI_APPLICATION modules built
with ShellCEntryLib would depend on the shell to function, and the shell
was not available when the boot manager launched the apps via SysPrep.

This argument actually makes sense to me; I'm not positing that
ShellCEntryLib apps "should" work in the above situation. The problem is
that the functional alternative (UefiApplicationEntryPoint) has been
super difficult to learn about (from my personal POV anyway).

> And don't forget most platforms ship without a built-in Shell,

correct

> so there would be no way of running ... anything ... if that was true.

It would be more precise to put this as,

  there would be no way of running any *application*, built with *edk2*,
  directly from the *boot manager*

and that had been my exact mental image until I read this thread.

> Also, it's pretty much the whole point of gnu-efi (combined with doing
> it directly in a normal POSIX environment).

I agree 100%, and I didn't forget about shim, grub, or gnu-efi -- please
note the emphasis on "edk2" in my above reformulation.

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

Re: [edk2] [PATCH 1/4] OvmfPkg: introduce ACPI Test Support data structure and GUID

[Igor Mammedov]

On Sun, 25 Nov 2018 11:01:49 +0100
Laszlo Ersek  wrote:

> QEMU's test suite includes a set of cases called "BIOS tables test". Among
> other things, it locates the RSD PTR ACPI table in guest RAM, and then
> (chasing pointers to other ACPI tables) performs various sanity checks on
> the QEMU-generated and firmware-installed tables.
> 
> Currently this set of test cases doesn't work with UEFI guests, because
> the ACPI spec's definition for locating the RSD PTR in UEFI guests is a
> lot harder to implement from the hypervisor side -- just with raw guest
> memory access -- than it is when scraping the memory of BIOS guests.
> 
> Introduce a signature GUID, and a small, MB-aligned structure, identified
> by the GUID. The hypervisor should loop over all MB-aligned pages in guest
> RAM until one matches the signature GUID at offset 0, at which point the
> hypervisor can fetch the RSDP address field(s) from the structure.
> 
> QEMU's test logic currently spins on a pre-determined guest address, until
> that address assumes a magic value. The method described in this patch is
> conceptually the same ("busy loop until match is found"), except there is
> no hard-coded address. This plays a lot more nicely with UEFI guest
> firmware (we'll be able to use the normal page allocation UEFI service).
> Given the size of EFI_GUID (16 bytes -- 128 bits), mismatches should be
> astronomically unlikely. In addition, given the typical guest RAM size for
> such tests (128 MB), there are 128 locations to check in one iteration of
> the "outer" loop, which shouldn't introduce an intolerable delay after the
> guest stores the RSDP address(es), and then the GUID.
> 
> The GUID that the hypervisor should search for is
> 
>   AB87A6B1-2034-BDA0-71BD-375007757785
> 
> Expressed as a byte array:
> 
>   {
> 0xb1, 0xa6, 0x87, 0xab,
> 0x34, 0x20,
> 0xa0, 0xbd,
> 0x71, 0xbd, 0x37, 0x50, 0x07, 0x75, 0x77, 0x85
>   }
> 
> Note that in the patch, we define "gAcpiTestSupportGuid" with all bits
> inverted. This is a simple method to prevent the UEFI binaries that
> incorporate "gAcpiTestSupportGuid" from matching the actual GUID in guest
> RAM.
> 
> Cc: Anthony Perard 
> Cc: Ard Biesheuvel 
> Cc: Drew Jones 
> Cc: Igor Mammedov 
> Cc: Jordan Justen 
> Cc: Julien Grall 
> Cc: Philippe Mathieu-Daudé 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
>  OvmfPkg/OvmfPkg.dec|  1 +
>  OvmfPkg/Include/Guid/AcpiTestSupport.h | 67 
>  2 files changed, 68 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 7666297cf8f1..e8c7d9423f43 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -73,11 +73,12 @@ [LibraryClasses]
>  [Guids]
>gUefiOvmfPkgTokenSpaceGuid  = {0x93bb96af, 0xb9f2, 0x4eb8, {0x94, 
> 0x62, 0xe0, 0xba, 0x74, 0x56, 0x42, 0x36}}
>gEfiXenInfoGuid = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 
> 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
>gOvmfPlatformConfigGuid = {0x7235c51c, 0x0c80, 0x4cab, {0x87, 
> 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}}
>gVirtioMmioTransportGuid= {0x837dca9e, 0xe874, 0x4d82, {0xb2, 
> 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}}
>gQemuRamfbGuid  = {0x557423a1, 0x63ab, 0x406c, {0xbe, 
> 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
>gXenBusRootDeviceGuid   = {0xa732241f, 0x383d, 0x4d9c, {0x8a, 
> 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
>gRootBridgesConnectedEventGroupGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90, 
> 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
> +  gAcpiTestSupportGuid= {0x5478594e, 0xdfcb, 0x425f, {0x8e, 
> 0x42, 0xc8, 0xaf, 0xf8, 0x8a, 0x88, 0x7a}}
>  
>  [Protocols]
>gVirtioDeviceProtocolGuid   = {0xfa920010, 0x6785, 0x4941, {0xb6, 
> 0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}}
> diff --git a/OvmfPkg/Include/Guid/AcpiTestSupport.h 
> b/OvmfPkg/Include/Guid/AcpiTestSupport.h
> new file mode 100644
> index ..8526f2bfb391
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/AcpiTestSupport.h
> @@ -0,0 +1,67 @@
> +/** @file
> +  Expose the address(es) of the ACPI RSD PTR table(s) in a MB-aligned 
> structure
> +  to the hypervisor.
> +
> +  The hypervisor locates the MB-aligned structure based on the signature GUID
> +  that is at offset 0 in the structure. Once the RSD PTR address(es) are
> +  retrieved, the hypervisor may perform various ACPI checks.
> +
> +  This feature is a development aid, for supporting ACPI table unit tests in
> +  hypervisors. Do not enable in production builds.
> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License that accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS 

Re: [edk2] Pkcs7 crypto verification without openSSL

[Ard Biesheuvel]

On Mon, 3 Dec 2018 at 13:55, Tomas Pilar (tpilar)  wrote:
>
>
>
> On 03/12/2018 12:40, Ard Biesheuvel wrote:
> > On Wed, 28 Nov 2018 at 18:40, Tomas Pilar (tpilar)
> >  wrote:
> >> Hi,
> >>
> >> Are there any plans for a crypto library that does not pull in openSSL? 
> >> When I try to add BaseCryptLib to be able to use FmpAuthenticationLib, my 
> >> driver size baloons significantly (increase of ~0x3) and it seems like 
> >> a basic public SHA256 crypto check library should not be _that_ large?
> > Well, I'd expect the code size to come from the asymmetric crypto, not
> > from the SHA256 hash code. Which FmpAuthenticationLib are you using?
> Yes, that makes sense. I am using the FmpAuthenticationLibPkcs7 from 
> SecurityPkg which pulls in BaseCryptLib. I would assume that the linker only 
> links the functions which are referenced so it should not pull in the entire 
> library but I expect some overgeneric helpers might be quite large.
>

Just the arbitrary precision integer library needed for the modular
exponentiation produces a fair chunk of code.

You can check the .map file in the Build/ directory of your driver
where all the memory is going, but all of the bn_xxx objects are
probably required.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/4] ArmVirtQemu: unmap page #0 to catch NULL pointer dereferences

[Ard Biesheuvel]

On Fri, 30 Nov 2018 at 12:28, Ard Biesheuvel  wrote:
>
> Rationale in patch #4. Patch #3 is a prerequisite patch that ensures
> that we no longer need page #0 to be mapped for the NOR flash driver
> to be able to expose it as a read/write block device.
>
> Patches #1 and #2 are fixes for the ARM version of the ArmMmuLib driver
> for bugs that get triggered by these changes.
>
> Cc: Leif Lindholm 
> Cc: Laszlo Ersek 
> Cc: Eric Auger 
> Cc: Andrew Jones 
> Cc: Philippe Mathieu-Daude 
>
> Ard Biesheuvel (4):
>   ArmPkg/ArmMmuLib ARM: handle unmapped section in GetMemoryRegion()
>   ArmPkg/ArmMmuLib ARM: handle unmapped sections when updating
> permissions
>   ArmVirtPkg/NorFlashQemuLib: disregard our primary FV
>   ArmVirtPkg/QemuVirtMemInfoLib: trim the MMIO region mapping
>

Pushed as a2df8587bf7a..51bb05c79595

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

Re: [edk2] [Patch v1 1/1] BaseTools: create and use a standard shared variable for '*'

[Carsey, Jaben]

I was trying to change all use of *, without regard to the usage of it.

Do you think that mathematical * should not be changed?

-Jaben

> -Original Message-
> From: Zhu, Yonghong
> Sent: Sunday, December 02, 2018 6:31 PM
> To: Carsey, Jaben ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zhu, Yonghong
> 
> Subject: RE: [Patch v1 1/1] BaseTools: create and use a standard shared
> variable for '*'
> Importance: High
> 
> Hi Jaben,
> 
> In this patch, it also changed the mathematics multiplicative '*' (used in
> expression) to  TAB_STAR, is it by on purpose?
> Eg:
> -NonLetterOpLst = ['+', '-', '*', '/', '%', '&', '|', '^', '~', '<<', 
> '>>', '!', '=', '>', '<',
> '?', ':']
> +NonLetterOpLst = ['+', '-', TAB_STAR, '/', '%', '&', '|', '^', '~', 
> '<<', '>>', '!', '=',
> '>', '<', '?', ':']
> 
> Best Regards,
> Zhu Yonghong
> 
> -Original Message-
> From: Carsey, Jaben
> Sent: Friday, November 16, 2018 11:40 PM
> To: edk2-devel@lists.01.org
> Cc: Zhu, Yonghong ; Gao, Liming
> 
> Subject: [Patch v1 1/1] BaseTools: create and use a standard shared variable
> for '*'
> 
> add a variable for the string '*' and then use it instead of lots of '*'
> 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jaben Carsey 
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 54 ++---
> ---
>  BaseTools/Source/Python/AutoGen/BuildEngine.py | 10 ++--
>  BaseTools/Source/Python/BPDG/GenVpd.py | 14 ++---
>  BaseTools/Source/Python/Common/DataType.py |  1 +
>  BaseTools/Source/Python/Common/Expression.py   |  4 +-
>  BaseTools/Source/Python/Common/Misc.py |  2 +-
>  BaseTools/Source/Python/Common/ToolDefClassObject.py   | 23 +
>  BaseTools/Source/Python/Common/VpdInfoFile.py  |  8 +--
>  BaseTools/Source/Python/GenFds/FdfParser.py|  5 +-
>  BaseTools/Source/Python/GenFds/GenFds.py   |  2 +-
>  BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py |  8 +--
>  BaseTools/Source/Python/GenFds/Section.py  |  2 +-
>  BaseTools/Source/Python/Workspace/DscBuildData.py  |  6 +--
>  13 files changed, 70 insertions(+), 69 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index f3560bfc787d..25417c447061 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -1438,7 +1438,7 @@ class PlatformAutoGen(AutoGen):
>  PcdValue = Sku.DefaultValue
>  if PcdValue == "":
>  PcdValue  = Pcd.DefaultValue
> -if Sku.VpdOffset != '*':
> +if Sku.VpdOffset != TAB_STAR:
>  if PcdValue.startswith("{"):
>  Alignment = 8
>  elif PcdValue.startswith("L"):
> @@ -1462,7 +1462,7 @@ class PlatformAutoGen(AutoGen):
>  VpdFile.Add(Pcd, SkuName, Sku.VpdOffset)
>  SkuValueMap[PcdValue].append(Sku)
>  # if the offset of a VPD is *, then it need to be 
> fixed up by third
> party tool.
> -if not NeedProcessVpdMapFile and Sku.VpdOffset == 
> "*":
> +if not NeedProcessVpdMapFile and Sku.VpdOffset ==
> TAB_STAR:
>  NeedProcessVpdMapFile = True
>  if self.Platform.VpdToolGuid is None or
> self.Platform.VpdToolGuid == '':
>  EdkLogger.error("Build", FILE_NOT_FOUND, \ 
> @@ -1522,7
> +1522,7 @@ class PlatformAutoGen(AutoGen):
>  PcdValue = Sku.DefaultValue
>  if PcdValue == "":
>  PcdValue  = DscPcdEntry.DefaultValue
> -if Sku.VpdOffset != '*':
> +if Sku.VpdOffset != TAB_STAR:
>  if PcdValue.startswith("{"):
>  Alignment = 8
>  elif PcdValue.startswith("L"):
> @@ -1545,7 +1545,7 @@ class PlatformAutoGen(AutoGen):
>  SkuValueMap[PcdValue] = []
>  VpdFile.Add(DscPcdEntry, SkuName, 
> Sku.VpdOffset)
>  SkuValueMap[PcdValue].append(Sku)
> -if not NeedProcessVpdMapFile and 
> Sku.VpdOffset == "*":
> +if not NeedProcessVpdMapFile and 
> Sku.VpdOffset ==
> TAB_STAR:
>  NeedProcessVpdMapFile = True
>  if DscPcdEntry.DatumType == TAB_VOID and
> PcdValue.startswith("L"):
>  

Re: [edk2] [RFC] Proposal to add edk2-apps repository

[Leif Lindholm]

On Mon, Dec 03, 2018 at 03:11:33PM +0100, Laszlo Ersek wrote:
> On 11/30/18 07:03, Andrew Fish wrote:
> > Mike,
> >
> > As Krishna points out there are flavors of Apps. Do we want to have
> > different packages for different flavor of apps, or different dirs in
> > a more generic App package? Maybe we should define classes of UEFI
> > Applications in the README.md and give them a place to live.
> 
> In my opinion, this is absolutely the first step that should be done.

Yes please.

> Personally, I've just learned, from this thread, that there are *three*
> (not two) UEFI application entry point types that edk2 supports.
> 
> * I've always known about main() -- libc app --, and ShellAppMain() --
>   shell app. I've always known these because I read about them in
>   "AppPkg/ReadMe.txt" and "StdLib/ReadMe.txt" years ago. In particular,
>   compare the description of "Hello" and "Main".
> 
> * And now MdeModulePkg/Application has been mentioned, in this thread,
>   where I see UefiMain() as the entry point.

Well, these are defined by the application .inf though, so "UefiMain"
is just a common name picked. MdeModulePkg/Application also has
applications with entry points called 'BootManagerMenuEntry',
'SmiHandlerProfileInfoEntrypoint' and 'InitializeUserInterface'. (The
latter being UiApp.)

> I don't recall reading about UefiMain() or UefiApplicationEntryPoint on
> this list. On the other hand, I remember several discussions where
> people asked if they could write an application and invoke it from
> SysPrep or similar, and the answer has always been, "oh sorry you
> can't do that, because the lowest level you can go is ShellAppMain(),
> and that won't work from SysPrep".

Huh? Who claimed that? Where?

(I suspect the meaning of "Application" has taken on some
overly-specific meaning for someone if they have claimed such - going
back to how having a proper definition is clearly quite important.)

Of course they can. Any EDK2 image itself contains a bunch of .efi
executables - and there is nothing preventing you from invoking those
from the shell.

And don't forget most platforms ship without a built-in Shell, so
there would be no way of running ... anything ... if that was true.

Also, it's pretty much the whole point of gnu-efi (combined with doing
it directly in a normal POSIX environment).

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

Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

[Udit Kumar]

Hi Ard 

> -Original Message-
> From: Ard Biesheuvel 
> Sent: Monday, December 3, 2018 7:35 PM
> To: Laszlo Ersek 
> Cc: Udit Kumar ; Andrew Fish ; Leif
> Lindholm ; Ruiyu Ni ; edk2-
> de...@lists.01.org; Zeng, Star 
> Subject: Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot
> 
> On Mon, 3 Dec 2018 at 14:45, Laszlo Ersek  wrote:
> >
> > On 12/03/18 11:03, Udit Kumar wrote:
> > > Hi Laszlo
> > >
> > >> Are you using the latest edk2?
> > > Yes, when I hit the bug, first step was to move to latest edk2. But this 
> > > didn't
> helped.
> > >
> > > I am not sure, why we are keeping PL011 Fifo mode off in default
> configuration.
> >
> > Sorry, I don't know: although the subject logic has gone through
> > several updates over time, the default that you point out seems to go
> > back to commit 051e63bb551a ("ArmPlatformPkg/PL011Uart: Allowed to
> > change UART settings in its initialization function", 2012-02-29). And
> > that commit doesn't explain the default.
> >
> 
> The default setting for
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth is 1 not 0, and so
> you will end up with a 1 character FIFO setting. I wonder if we could improve

Setting this Pcd to zero will fix problem for me, 
With  PcdUartDefaultReceiveFifoDepth as 1, we are disabling the Fifo at IP 
level whereas for 0 we are enabling IP fifo.

> PL011UartInitializePort () in PL011UartLib to deal with this better.

Suggest, if IP level Fifo can be kept enabled, always ?

> Note that this also affects platforms that can switch between DT and ACPI 
> boot:
> the same PL011 is described as a 'dumb' SBSA uart in the latter case, and so 
> it is
> up to the firmware to configure the UART FIFO correctly, or you will not be 
> able
> to use the UART under the ACPI OS.

For my ACPI boot (without SPCR table), I hit the same issue.

OS  (Linux, not sure of other OSes) except FIFO to be enabled by 
firmware/boot-loader. 

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

[edk2] SCT bugzilla topic?

[Leif Lindholm]

Hi Eric, Supreeth, Mike,

I was looking to raise a feature request on UEFI SCT and didn't spot
such a product. Should we create one?

Or perhaps we should have an EDK2 Test product with an SCT component?

Regards,

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

Re: [edk2] [RFC] Proposal to add edk2-apps repository

[Laszlo Ersek]

On 11/30/18 05:57, Kinney, Michael D wrote:

> Virtual platforms such as Ovmf and EmulatorPkg should stay in edk2
> repo to support validation.

Yes, and ArmVirtPkg too.

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

Re: [edk2] [RFC] Proposal to add edk2-apps repository

[Laszlo Ersek]

On 11/30/18 07:03, Andrew Fish wrote:
> Mike,
>
> As Krishna points out there are flavors of Apps. Do we want to have
> different packages for different flavor of apps, or different dirs in
> a more generic App package? Maybe we should define classes of UEFI
> Applications in the README.md and give them a place to live.

In my opinion, this is absolutely the first step that should be done.

Personally, I've just learned, from this thread, that there are *three*
(not two) UEFI application entry point types that edk2 supports.

* I've always known about main() -- libc app --, and ShellAppMain() --
  shell app. I've always known these because I read about them in
  "AppPkg/ReadMe.txt" and "StdLib/ReadMe.txt" years ago. In particular,
  compare the description of "Hello" and "Main".

* And now MdeModulePkg/Application has been mentioned, in this thread,
  where I see UefiMain() as the entry point.

I don't recall reading about UefiMain() or UefiApplicationEntryPoint on
this list. On the other hand, I remember several discussions where
people asked if they could write an application and invoke it from
SysPrep or similar, and the answer has always been, "oh sorry you
can't do that, because the lowest level you can go is ShellAppMain(),
and that won't work from SysPrep".

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

Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

[Ard Biesheuvel]

On Mon, 3 Dec 2018 at 14:45, Laszlo Ersek  wrote:
>
> On 12/03/18 11:03, Udit Kumar wrote:
> > Hi Laszlo
> >
> >> Are you using the latest edk2?
> > Yes, when I hit the bug, first step was to move to latest edk2. But this 
> > didn't helped.
> >
> > I am not sure, why we are keeping PL011 Fifo mode off in default 
> > configuration.
>
> Sorry, I don't know: although the subject logic has gone through several
> updates over time, the default that you point out seems to go back to
> commit 051e63bb551a ("ArmPlatformPkg/PL011Uart: Allowed to change UART
> settings in its initialization function", 2012-02-29). And that commit
> doesn't explain the default.
>

The default setting for
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth is 1 not 0,
and so you will end up with a 1 character FIFO setting. I wonder if we
could improve PL011UartInitializePort () in PL011UartLib to deal with
this better.

Note that this also affects platforms that can switch between DT and
ACPI boot: the same PL011 is described as a 'dumb' SBSA uart in the
latter case, and so it is up to the firmware to configure the UART
FIFO correctly, or you will not be able to use the UART under the ACPI
OS.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [edk2-test][v2 Patch 0/3] Add VerifySignature() Test

[Laszlo Ersek]

On 12/03/18 08:53, Eric Jin wrote:
> This is the cover letter.
> The series of patches are listed below.
> 
>   uefi-sct/SctPkg:Format code style in PKCS7Verify
>   uefi-sct/SctPkg:Add VerifySignature() Func Test
>   uefi-sct/SctPkg:Add VerifySignature() Conf Test
> 
> 
>  .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |   93 +-
>  .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |  192 +--
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTest.inf   |  126 +-
>  .../BlackBoxTest/Pkcs7BBTestConformance.c  | 1305 +---
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c | 1568 
> +---
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestFunction.c |  553 ---
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |  458 +++---
>  .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.h |  248 ++--
>  8 files changed, 2689 insertions(+), 1854 deletions(-)
> 

A cover letter like this doesn't make any sense.

- Please enable shallow threading for git-send-email, so that the patch
emails are (direct) children of the cover letter. One of the goals of
the cover letter is to group the patches under a common parent message.

- The other purpose of cover letters is to summarize the goal of the
patch series (high level problem statement, chosen solution, maybe
mention one or two details if they are important).

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

Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

[Laszlo Ersek]

On 12/03/18 11:03, Udit Kumar wrote:
> Hi Laszlo 
> 
>> Are you using the latest edk2?
> Yes, when I hit the bug, first step was to move to latest edk2. But this 
> didn't helped.
> 
> I am not sure, why we are keeping PL011 Fifo mode off in default 
> configuration.  

Sorry, I don't know: although the subject logic has gone through several
updates over time, the default that you point out seems to go back to
commit 051e63bb551a ("ArmPlatformPkg/PL011Uart: Allowed to change UART
settings in its initialization function", 2012-02-29). And that commit
doesn't explain the default.

Thanks
Laszlo

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

Re: [edk2] [PATCH v2 4/4] ArmVirtPkg/QemuVirtMemInfoLib: trim the MMIO region mapping

[Laszlo Ersek]

On 11/30/18 12:28, Ard Biesheuvel wrote:
> QEMU/mach-virt is rather unhelpful when it comes to tracking down
> NULL pointer dereferences that occur while running in UEFI: since
> we have NOR flash mapped at address 0x0, inadvertent reads go
> unnoticed, and even most writes are silently dropped, unless you're
> unlucky and the instruction in question is one that KVM cannot
> emulate, in which case you end up with a QEMU crash like this:
> 
>   error: kvm run failed Function not implemented
>PC=00013f7ff804 X00=00013f7ab108 X01=0064
>   X02=00013f801988 X03=83c4 X04=
>   X05=9644 X06=fffd8270 X07=00013f7ab4a0
>   X08=0001 X09=00013f803b88 X10=00013f7e88d0
>   X11=0009 X12=00013f7ab554 X13=0008
>   X14=0002 X15= X16=
>   X17= X18= X19=
>   X20=00013f81c000 X21=00013f7ab170 X22=00013f81c000
>   X23=0918 X24=00013f407020 X25=00013f81c000
>   X26=00013f803530 X27=00013f802000 X28=00013f7ab270
>   X29=00013f7ab0d0 X30=00013f7fee10  SP=00013f7a6f30
>   PSTATE=83c5 N--- EL1h
> 
> and a warning in the host kernel log that load/store instruction
> decoding is not supported by KVM.
> 
> Given that the first page of the flash device is not actually
> used anyway, let's reduce the mappings of the peripheral space
> and the flash device (both of which cover page #0) to only cover
> what is actually required:
> 
>   ArmVirtQemu.fdf:
>   > 0x1000|0x001ff000
>   > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> 
>   ArmVirtQemuKernel.fdf:
>   > 0x8000|0x001f8000
>   > gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> 
> For ArmVirtQemu, the resulting virtual mapping looks roughly like:
> - [0, 4K)   : flash, unmapped
> - [4K, 2M)  : flash, mapped as WB+X RAM
> - [2M, 64M) : flash, unmapped
> - [64M, 128M)   : varstore flash, will be mapped by the NOR flash driver
> - [128M, 256M)  : peripherals, mapped as device
> - [256M, 1GB)   : 32-bit MMIO aperture, translated IO aperture, ECAM,
>   will be mapped by the PCI host bridge driver
> - [1GB, ...): RAM, mapped.
> 
> After this change, any inadvertent read or write from/to the first
> physical page will trigger a translation fault inside the guest,
> regardless of the nature of the instruction, without crashing QEMU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf|  4 ++--
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf |  2 ++
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c  | 23 
> ++--
>  3 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf 
> b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> index 5c5b841051ad..b6abc52531a8 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> @@ -39,9 +39,9 @@ [LibraryClasses]
>PcdLib
>  
>  [Pcd]
> -  gArmTokenSpaceGuid.PcdFdBaseAddress
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
>gArmTokenSpaceGuid.PcdSystemMemoryBase
>gArmTokenSpaceGuid.PcdSystemMemorySize
>  
>  [FixedPcd]
> -  gArmTokenSpaceGuid.PcdFdSize
> +  gArmTokenSpaceGuid.PcdFvSize
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf 
> b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> index d12089760b22..16802c5c414b 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> @@ -43,9 +43,11 @@ [LibraryClasses]
>  
>  [Pcd]
>gArmTokenSpaceGuid.PcdFdBaseAddress
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
>gArmTokenSpaceGuid.PcdSystemMemoryBase
>gArmTokenSpaceGuid.PcdSystemMemorySize
>  
>  [FixedPcd]
>gArmTokenSpaceGuid.PcdFdSize
> +  gArmTokenSpaceGuid.PcdFvSize
>gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c 
> b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> index 0285a11b1d77..a26b2fbad9be 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> @@ -21,6 +21,15 @@
>  // Number of Virtual Memory Map Descriptors
>  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  5
>  
> +//
> +// mach-virt's core peripherals such as the UART, the GIC and the RTC are
> +// all mapped in the 'miscellaneous device I/O' region, which we just map
> +// in its entirety rather than device by device. Note that it does not
> +// cover any of the NOR flash 

Re: [edk2] [PATCH v2 3/4] ArmVirtPkg/NorFlashQemuLib: disregard our primary FV

[Laszlo Ersek]

On 11/30/18 12:28, Ard Biesheuvel wrote:
> The primary FV contains the firmware boot image, which is not
> runtime updatable in our case. So exposing it to the NOR flash
> driver is undesirable, since it may attempt to modify the NOR
> flash contents. It is also rather pointless, since we don't
> keep anything there that we care to expose. (the SEC and PEI
> phase modules are not executable from DXE context, and the
> contents of the embedded DXE phase FV are exposed by the DXE
> core directly via the FVB2 protocol)
> 
> So let's disregard the NOR flash block that covers the primary
> FV.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf |  5 +
>  ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c   | 13 +++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf 
> b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> index d86ff36dbd58..c5752a243e6b 100644
> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
> @@ -28,6 +28,7 @@ [Sources.common]
>  [Packages]
>MdePkg/MdePkg.dec
>ArmPlatformPkg/ArmPlatformPkg.dec
> +  ArmPkg/ArmPkg.dec
>ArmVirtPkg/ArmVirtPkg.dec
>  
>  [LibraryClasses]
> @@ -40,3 +41,7 @@ [Protocols]
>  
>  [Depex]
>gFdtClientProtocolGuid
> +
> +[Pcd]
> +  gArmTokenSpaceGuid.PcdFvBaseAddress
> +  gArmTokenSpaceGuid.PcdFvSize
> diff --git a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c 
> b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> index 2678f57eaaad..d238e39a59f1 100644
> --- a/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> +++ b/ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.c
> @@ -75,13 +75,22 @@ NorFlashPlatformGetDevices (
>Size = SwapBytes64 (ReadUnaligned64 ((VOID *)[2]));
>Reg += 4;
>  
> +  PropSize -= 4 * sizeof (UINT32);
> +
> +  //
> +  // Disregard any flash devices that overlap with the primary FV.
> +  // The firmware is not updatable from inside the guest anyway.
> +  //
> +  if ((PcdGet64 (PcdFvBaseAddress) + PcdGet32 (PcdFvSize) > Base) &&
> +  (Base + Size) > PcdGet64 (PcdFvBaseAddress)) {
> +continue;
> +  }
> +
>mNorFlashDevices[Num].DeviceBaseAddress = (UINTN)Base;
>mNorFlashDevices[Num].RegionBaseAddress = (UINTN)Base;
>mNorFlashDevices[Num].Size  = (UINTN)Size;
>mNorFlashDevices[Num].BlockSize = QEMU_NOR_BLOCK_SIZE;
>Num++;
> -
> -  PropSize -= 4 * sizeof (UINT32);
>  }
>}
>  
> 

Reviewed-by: Laszlo Ersek 

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

Re: [edk2] [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN

[Laszlo Ersek]

On 11/30/18 23:45, Ard Biesheuvel wrote:
> The maximum value that can be represented by the native word size
> of the *target* should be irrelevant when compiling tools that
> run on the build *host*. So drop the definition of MAX_UINTN, now
> that we no longer use it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Jaben Carsey 
> ---
>  BaseTools/Source/C/Common/CommonLib.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.h 
> b/BaseTools/Source/C/Common/CommonLib.h
> index 6930d9227b87..b1c6c00a3478 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  
>  #define MAX_LONG_FILE_PATH 500
>  
> -#define MAX_UINTN MAX_ADDRESS
>  #define MAX_UINT64 ((UINT64)0xULL)
>  #define MAX_UINT16  ((UINT16)0x)
>  #define MAX_UINT8   ((UINT8)0xFF)
> 

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

Re: [edk2] [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines

[Laszlo Ersek]

On 11/30/18 23:45, Ard Biesheuvel wrote:
> Parsing a string into an integer variable of the native word size
> is not defined for the BaseTools, since the same tools may be used
> to build firmware for different targets with different native word
> sizes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/Common/CommonLib.h |  24 ---
>  BaseTools/Source/C/Common/CommonLib.c | 174 +---
>  2 files changed, 5 insertions(+), 193 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.h 
> b/BaseTools/Source/C/Common/CommonLib.h
> index fa10fac4682a..6930d9227b87 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -250,16 +250,6 @@ StrSize (
>CONST CHAR16  *String
>);
>  
> -UINTN
> -StrHexToUintn (
> -  CONST CHAR16  *String
> -  );
> -
> -UINTN
> -StrDecimalToUintn (
> -  CONST CHAR16  *String
> -  );
> -
>  UINT64
>  StrHexToUint64 (
>CONST CHAR16 *String
> @@ -277,13 +267,6 @@ StrHexToUint64S (
>  UINT64 *Data
>);
>  
> -RETURN_STATUS
> -StrHexToUintnS (
> -CONST CHAR16 *String,
> - CHAR16 **EndPointer,  OPTIONAL
> - UINTN  *Data
> -  );
> -
>  RETURN_STATUS
>  StrDecimalToUint64S (
>  CONST CHAR16 *String,
> @@ -291,13 +274,6 @@ StrDecimalToUint64S (
>   UINT64 *Data
>);
>  
> -RETURN_STATUS
> -StrDecimalToUintnS (
> -CONST CHAR16 *String,
> - CHAR16 **EndPointer,  OPTIONAL
> - UINTN  *Data
> -  );
> -
>  VOID *
>  ReallocatePool (
> UINTN  OldSize,
> diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> b/BaseTools/Source/C/Common/CommonLib.c
> index 4a28f635f3a8..42dfa821624d 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -882,72 +882,6 @@ InternalSafeStringNoStrOverlap (
>return !InternalSafeStringIsOverlap (Str1, Size1 * sizeof(CHAR16), Str2, 
> Size2 * sizeof(CHAR16));
>  }
>  
> -RETURN_STATUS
> -StrDecimalToUintnS (
> -CONST CHAR16 *String,
> - CHAR16 **EndPointer,  OPTIONAL
> - UINTN  *Data
> -  )
> -{
> -  ASSERT (((UINTN) String & BIT0) == 0);
> -
> -  //
> -  // 1. Neither String nor Data shall be a null pointer.
> -  //
> -  SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
> -  SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
> -
> -  //
> -  // 2. The length of String shall not be greater than RSIZE_MAX.
> -  //
> -  if (RSIZE_MAX != 0) {
> -SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= 
> RSIZE_MAX), RETURN_INVALID_PARAMETER);
> -  }
> -
> -  if (EndPointer != NULL) {
> -*EndPointer = (CHAR16 *) String;
> -  }
> -
> -  //
> -  // Ignore the pad spaces (space or tab)
> -  //
> -  while ((*String == L' ') || (*String == L'\t')) {
> -String++;
> -  }
> -
> -  //
> -  // Ignore leading Zeros after the spaces
> -  //
> -  while (*String == L'0') {
> -String++;
> -  }
> -
> -  *Data = 0;
> -
> -  while (InternalIsDecimalDigitCharacter (*String)) {
> -//
> -// If the number represented by String overflows according to the range
> -// defined by UINTN, then MAX_UINTN is stored in *Data and
> -// RETURN_UNSUPPORTED is returned.
> -//
> -if (*Data > ((MAX_UINTN - (*String - L'0')) / 10)) {
> -  *Data = MAX_UINTN;
> -  if (EndPointer != NULL) {
> -*EndPointer = (CHAR16 *) String;
> -  }
> -  return RETURN_UNSUPPORTED;
> -}
> -
> -*Data = *Data * 10 + (*String - L'0');
> -String++;
> -  }
> -
> -  if (EndPointer != NULL) {
> -*EndPointer = (CHAR16 *) String;
> -  }
> -  return RETURN_SUCCESS;
> -}
> -
>  /**
>Convert a Null-terminated Unicode decimal string to a value of type UINT64.
>  
> @@ -1064,9 +998,9 @@ StrDecimalToUint64S (
>  
>  /**
>Convert a Null-terminated Unicode hexadecimal string to a value of type
> -  UINTN.
> +  UINT64.
>  
> -  This function outputs a value of type UINTN by interpreting the contents of
> +  This function outputs a value of type UINT64 by interpreting the contents 
> of
>the Unicode string specified by String as a hexadecimal number. The format 
> of
>the input Unicode string String is:
>  
> @@ -1091,8 +1025,8 @@ StrDecimalToUint64S (
>  
>If String has no valid hexadecimal digits in the above format, then 0 is
>stored at the location pointed to by Data.
> -  If the number represented by String exceeds the range defined by UINTN, 
> then
> -  MAX_UINTN is stored at the location pointed to by Data.
> +  If the number represented by String exceeds the range defined by UINT64, 
> then
> +  MAX_UINT64 is stored at the location pointed to by Data.
>  
>If EndPointer is not NULL, a pointer to the 

Re: [edk2] [PATCH v2 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size

[Laszlo Ersek]

On 11/30/18 23:45, Ard Biesheuvel wrote:
> Replace the default size limit of IsDevicePathValid() with a value
> that does not depend on the native word size of the build host.
> 
> 64 KB seems sufficient as the upper bound of a device path handled
> by UEFI.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Jaben Carsey 
> ---
>  BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c 
> b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> index d4ec2742b7c8..ba7f83e53070 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> @@ -62,7 +62,7 @@ IsDevicePathValid (
>ASSERT (DevicePath != NULL);
>  
>if (MaxSize == 0) {
> -MaxSize = MAX_UINTN;
> +MaxSize = MAX_UINT16;
>   }
>  
>//
> @@ -78,7 +78,7 @@ IsDevicePathValid (
>return FALSE;
>  }
>  
> -if (NodeLength > MAX_UINTN - Size) {
> +if (NodeLength > MAX_UINT16 - Size) {
>return FALSE;
>  }
>  Size += NodeLength;
> 

I'm somewhat undecided about this patch.

(1) IsDevicePathValid() also exists in:

- MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
- MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c

Both have:

  if (MaxSize == 0) {
MaxSize = MAX_UINTN;
  }

Relative to those, this change departs quite strongly.


(2) In addition, a single device path node may extend up to 64KB. That
would be pathologic, yes, but the option is there.


... Of course, we are discussing theoretical limits. Still I'd feel more
comfortable with MAX_UINT32. Lifting the limit from 64K to 4G wouldn't
cost us anything (in development effort), it would be a no-op on 32-bit
build hosts, it would be a theoretical-only change on 64-bit build
hosts, and it would leave us with a larger "safety margin".

I won't insist, but I thought I should raise this. (Sorry if this has
been discussed under v1 already.) If you agree, no need to repost (from
my side anyway) just for this.

With or without the update:

Reviewed-by: Laszlo Ersek 

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

Re: [edk2] Pkcs7 crypto verification without openSSL

[Tomas Pilar (tpilar)]

On 03/12/2018 12:40, Ard Biesheuvel wrote:
> On Wed, 28 Nov 2018 at 18:40, Tomas Pilar (tpilar)
>  wrote:
>> Hi,
>>
>> Are there any plans for a crypto library that does not pull in openSSL? When 
>> I try to add BaseCryptLib to be able to use FmpAuthenticationLib, my driver 
>> size baloons significantly (increase of ~0x3) and it seems like a basic 
>> public SHA256 crypto check library should not be _that_ large?
> Well, I'd expect the code size to come from the asymmetric crypto, not
> from the SHA256 hash code. Which FmpAuthenticationLib are you using?
Yes, that makes sense. I am using the FmpAuthenticationLibPkcs7 from 
SecurityPkg which pulls in BaseCryptLib. I would assume that the linker only 
links the functions which are referenced so it should not pull in the entire 
library but I expect some overgeneric helpers might be quite large.

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

Re: [edk2] [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines

[Laszlo Ersek]

On 11/30/18 23:45, Ard Biesheuvel wrote:
> Replace invocations of StrHexToUintn() with StrHexToUint64(), so
> that we can drop the former.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Jaben Carsey 
> ---
>  BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c 
> b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> index 555efa1acdde..6151926af9aa 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> @@ -520,7 +520,7 @@ EisaIdFromText (
>return (((Text[0] - 'A' + 1) & 0x1f) << 10)
> + (((Text[1] - 'A' + 1) & 0x1f) <<  5)
> + (((Text[2] - 'A' + 1) & 0x1f) <<  0)
> -   + (UINT32) (StrHexToUintn ([3]) << 16)
> +   + (UINT32) (StrHexToUint64 ([3]) << 16)
> ;
>  }
>  

This (theoretically) introduces a bit-shift on a UINT64 value, which
might result in a compiler intrinsic call on 32-bit build hosts. But
that's fine, because this is not firmware code, but hosted code.

> @@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
>  
>Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
>while (Index-- != 0) {
> -Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (, L'-'));
> +Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (, 
> L'-'));
>}
>  
>return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
> 

Reviewed-by: Laszlo Ersek 

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

Re: [edk2] [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()

[Laszlo Ersek]

On 11/30/18 23:45, Ard Biesheuvel wrote:
> Don't use the native word size string to number parsing routines,
> but instead, use the 64-bit one and cast to UINTN.
> 
> Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
> which takes care to use Strtoi64 () unless it assumes the value fits
> in 32-bit, so this change is a no-op even on 32-bit build hosts.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Jaben Carsey 
> ---
>  BaseTools/Source/C/Common/CommonLib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> b/BaseTools/Source/C/Common/CommonLib.c
> index 7c559bc3c222..4a28f635f3a8 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -2252,9 +2252,9 @@ Strtoi (
>)
>  {
>if (IsHexStr (Str)) {
> -return StrHexToUintn (Str);
> +return (UINTN)StrHexToUint64 (Str);
>} else {
> -return StrDecimalToUintn (Str);
> +return (UINTN)StrDecimalToUint64 (Str);
>}
>  }
>  
> 

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

Re: [edk2] [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Laszlo Ersek]

On 11/30/18 23:45, Ard Biesheuvel wrote:
> In the context of the BaseTools, there is no such thing as a native word
> size, given that the same set of tools may be used to build a firmware
> image consisting of both 32-bit and 64-bit modules.
> 
> So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> types instead of UINTN types when parsing strings.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/Common/CommonLib.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> b/BaseTools/Source/C/Common/CommonLib.c
> index 618aadac781a..7c559bc3c222 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1785,7 +1785,7 @@ StrToIpv4Address (
>  {
>RETURN_STATUS  Status;
>UINTN  AddressIndex;
> -  UINTN  Uintn;
> +  UINT64 Uint64;
>EFI_IPv4_ADDRESS   LocalAddress;
>UINT8  LocalPrefixLength;
>CHAR16 *Pointer;
> @@ -1812,7 +1812,7 @@ StrToIpv4Address (
>  //
>  // Get D or P.
>  //
> -Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, , );
> +Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, , 
> );
>  if (RETURN_ERROR (Status)) {
>return RETURN_UNSUPPORTED;
>  }
> @@ -1820,18 +1820,18 @@ StrToIpv4Address (
>//
>// It's P.
>//
> -  if (Uintn > 32) {
> +  if (Uint64 > 32) {
>  return RETURN_UNSUPPORTED;
>}
> -  LocalPrefixLength = (UINT8) Uintn;
> +  LocalPrefixLength = (UINT8) Uint64;
>  } else {
>//
>// It's D.
>//
> -  if (Uintn > MAX_UINT8) {
> +  if (Uint64 > MAX_UINT8) {
>  return RETURN_UNSUPPORTED;
>}
> -  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> +  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
>AddressIndex++;
>  }
>  
> @@ -1888,7 +1888,7 @@ StrToIpv6Address (
>  {
>RETURN_STATUS  Status;
>UINTN  AddressIndex;
> -  UINTN  Uintn;
> +  UINT64 Uint64;
>EFI_IPv6_ADDRESS   LocalAddress;
>UINT8  LocalPrefixLength;
>CONST CHAR16   *Pointer;
> @@ -1969,7 +1969,7 @@ StrToIpv6Address (
>  //
>  // Get X.
>  //
> -Status = StrHexToUintnS (Pointer, , );
> +Status = StrHexToUint64S (Pointer, , );
>  if (RETURN_ERROR (Status) || End - Pointer > 4) {
>//
>// Number of hexadecimal digit characters is no more than 4.
> @@ -1978,24 +1978,24 @@ StrToIpv6Address (
>  }
>  Pointer = End;
>  //
> -// Uintn won't exceed MAX_UINT16 if number of hexadecimal digit 
> characters is no more than 4.
> +// Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit 
> characters is no more than 4.
>  //
>  ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
> -LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
> -LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
> +LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
> +LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
>  AddressIndex += 2;
>} else {
>  //
>  // Get P, then exit the loop.
>  //
> -Status = StrDecimalToUintnS (Pointer, , );
> -if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
> +Status = StrDecimalToUint64S (Pointer, , );
> +if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
>//
>// Prefix length should not exceed 128.
>//
>return RETURN_UNSUPPORTED;
>  }
> -LocalPrefixLength = (UINT8) Uintn;
> +LocalPrefixLength = (UINT8) Uint64;
>  Pointer = End;
>  break;
>}
> 

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

Re: [edk2] Pkcs7 crypto verification without openSSL

[Ard Biesheuvel]

On Wed, 28 Nov 2018 at 18:40, Tomas Pilar (tpilar)
 wrote:
>
> Hi,
>
> Are there any plans for a crypto library that does not pull in openSSL? When 
> I try to add BaseCryptLib to be able to use FmpAuthenticationLib, my driver 
> size baloons significantly (increase of ~0x3) and it seems like a basic 
> public SHA256 crypto check library should not be _that_ large?

Well, I'd expect the code size to come from the asymmetric crypto, not
from the SHA256 hash code. Which FmpAuthenticationLib are you using?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/2] Update CPUID related definition.

[Laszlo Ersek]

On 12/03/18 07:30, Eric Dong wrote:
> Update CPUID definition to follow SDM 2018'11 version, changes Include:
> 1. Add new fields to the existed data structure, impact CPUIDs include:
>   1. CPUID_THERMAL_POWER_MANAGEMENT 0x06
>CPUID_THERMAL_POWER_MANAGEMENT_EAX
>   2. CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS0x07
>CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EBX
>CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX
>   3. CPUID_ARCHITECTURAL_PERFORMANCE_MONITORING  0x0A
>CPUID_ARCHITECTURAL_PERFORMANCE_MONITORING_EDX
>   4. CPUID_EXTENDED_STATE   0x0D
>CPUID_EXTENDED_STATE_MAIN_LEAF_EAX
>CPUID_EXTENDED_STATE_SUB_LEAF_ECX
>   5. CPUID_INTEL_RDT_ALLOCATION 0x10
>CPUID_INTEL_RDT_ALLOCATION_ENUMERATION_SUB_LEAF_EBX
>   6. CPUID_INTEL_SGX0x12
>CPUID_INTEL_SGX_CAPABILITIES_0_SUB_LEAF_EAX
> 
> 2. Add new data structures which not existed before, impact CPUID includes:
>   1. CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS0x07
>CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_EDX
> 
> 3. Remove fields which defined before, impact CPUID includes:
>   1. CPUID_INTEL_RDT_ALLOCATION 0x10
>CPUID_INTEL_RDT_ALLOCATION_L3_CACHE_SUB_LEAF
> 0x01
>  CPUID_INTEL_RDT_ALLOCATION_L3_CACHE_SUB_LEAF_ECX
> 
> 4. Add new sub leaf which not existed before, impact CPUID includes:
>   1. CPUID_INTEL_RDT_ALLOCATION 0x10
>CPUID_INTEL_RDT_ALLOCATION_MEMORY_BANDWIDTH_SUB_LEAF
> 0x03
> 
> 5. Add new CPUIDs which not exist before, new CPUIDs include:
>   1. CPUID_DETERMINISTIC_ADDRESS_TRANSLATION_PARAMETERS 0x18
>   2. CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION 0x1F
> 
> Also update Cpuid application in UefiCpuPkg/Application folder.
> 
> Cc: Ruiyu Ni 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> 
> Eric Dong (2):
>   UefiCpuPkg/Cpuid.h: Sync CPUID definition to latest SDM.
>   UefiCpuPkg/Cpuid: Add code to support new definition.
> 
>  UefiCpuPkg/Application/Cpuid/Cpuid.c | 147 -
>  UefiCpuPkg/Include/Register/Cpuid.h  | 620 
> +--
>  2 files changed, 739 insertions(+), 28 deletions(-)
> 

Acked-by: Laszlo Ersek 

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

Re: [edk2] [PATCH v2 1/4] ArmPkg/ArmMmuLib ARM: handle unmapped section in GetMemoryRegion()

[Philippe Mathieu-Daudé]

On 30/11/18 12:28, Ard Biesheuvel wrote:
> GetMemoryRegion() is used to obtain the attributes of an existing
> mapping, to permit permission attribute changes to be optimized
> away if the attributes don't actually change.
> 
> The current ARM code assumes that a section mapping or a page mapping
> exists for any region passed into GetMemoryRegion(), but the region
> may be unmapped entirely, in which case the code will crash. So check
> if a section mapping exists before dereferencing it.

Good catch!

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> index 12ca5b26673e..3b29d33d0a9c 100644
> --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
> @@ -457,6 +457,9 @@ GetMemoryRegion (
>  
>// Get the section at the given index
>SectionDescriptor = FirstLevelTable[TableIndex];
> +  if (!SectionDescriptor) {
> +return EFI_NOT_FOUND;
> +  }
>  
>// If 'BaseAddress' belongs to the section then round it to the section 
> boundary
>if (((SectionDescriptor & TT_DESCRIPTOR_SECTION_TYPE_MASK) == 
> TT_DESCRIPTOR_SECTION_TYPE_SECTION) ||
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN

[Philippe Mathieu-Daudé]

On 30/11/18 23:45, Ard Biesheuvel wrote:
> The maximum value that can be represented by the native word size
> of the *target* should be irrelevant when compiling tools that
> run on the build *host*. So drop the definition of MAX_UINTN, now
> that we no longer use it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Jaben Carsey 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  BaseTools/Source/C/Common/CommonLib.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.h 
> b/BaseTools/Source/C/Common/CommonLib.h
> index 6930d9227b87..b1c6c00a3478 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  
>  #define MAX_LONG_FILE_PATH 500
>  
> -#define MAX_UINTN MAX_ADDRESS
>  #define MAX_UINT64 ((UINT64)0xULL)
>  #define MAX_UINT16  ((UINT16)0x)
>  #define MAX_UINT8   ((UINT8)0xFF)
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines

[Philippe Mathieu-Daudé]

On 30/11/18 23:45, Ard Biesheuvel wrote:
> Parsing a string into an integer variable of the native word size
> is not defined for the BaseTools, since the same tools may be used
> to build firmware for different targets with different native word
> sizes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  BaseTools/Source/C/Common/CommonLib.h |  24 ---
>  BaseTools/Source/C/Common/CommonLib.c | 174 +---
>  2 files changed, 5 insertions(+), 193 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.h 
> b/BaseTools/Source/C/Common/CommonLib.h
> index fa10fac4682a..6930d9227b87 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -250,16 +250,6 @@ StrSize (
>CONST CHAR16  *String
>);
>  
> -UINTN
> -StrHexToUintn (
> -  CONST CHAR16  *String
> -  );
> -
> -UINTN
> -StrDecimalToUintn (
> -  CONST CHAR16  *String
> -  );
> -
>  UINT64
>  StrHexToUint64 (
>CONST CHAR16 *String
> @@ -277,13 +267,6 @@ StrHexToUint64S (
>  UINT64 *Data
>);
>  
> -RETURN_STATUS
> -StrHexToUintnS (
> -CONST CHAR16 *String,
> - CHAR16 **EndPointer,  OPTIONAL
> - UINTN  *Data
> -  );
> -
>  RETURN_STATUS
>  StrDecimalToUint64S (
>  CONST CHAR16 *String,
> @@ -291,13 +274,6 @@ StrDecimalToUint64S (
>   UINT64 *Data
>);
>  
> -RETURN_STATUS
> -StrDecimalToUintnS (
> -CONST CHAR16 *String,
> - CHAR16 **EndPointer,  OPTIONAL
> - UINTN  *Data
> -  );
> -
>  VOID *
>  ReallocatePool (
> UINTN  OldSize,
> diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> b/BaseTools/Source/C/Common/CommonLib.c
> index 4a28f635f3a8..42dfa821624d 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -882,72 +882,6 @@ InternalSafeStringNoStrOverlap (
>return !InternalSafeStringIsOverlap (Str1, Size1 * sizeof(CHAR16), Str2, 
> Size2 * sizeof(CHAR16));
>  }
>  
> -RETURN_STATUS
> -StrDecimalToUintnS (
> -CONST CHAR16 *String,
> - CHAR16 **EndPointer,  OPTIONAL
> - UINTN  *Data
> -  )
> -{
> -  ASSERT (((UINTN) String & BIT0) == 0);
> -
> -  //
> -  // 1. Neither String nor Data shall be a null pointer.
> -  //
> -  SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
> -  SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
> -
> -  //
> -  // 2. The length of String shall not be greater than RSIZE_MAX.
> -  //
> -  if (RSIZE_MAX != 0) {
> -SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= 
> RSIZE_MAX), RETURN_INVALID_PARAMETER);
> -  }
> -
> -  if (EndPointer != NULL) {
> -*EndPointer = (CHAR16 *) String;
> -  }
> -
> -  //
> -  // Ignore the pad spaces (space or tab)
> -  //
> -  while ((*String == L' ') || (*String == L'\t')) {
> -String++;
> -  }
> -
> -  //
> -  // Ignore leading Zeros after the spaces
> -  //
> -  while (*String == L'0') {
> -String++;
> -  }
> -
> -  *Data = 0;
> -
> -  while (InternalIsDecimalDigitCharacter (*String)) {
> -//
> -// If the number represented by String overflows according to the range
> -// defined by UINTN, then MAX_UINTN is stored in *Data and
> -// RETURN_UNSUPPORTED is returned.
> -//
> -if (*Data > ((MAX_UINTN - (*String - L'0')) / 10)) {
> -  *Data = MAX_UINTN;
> -  if (EndPointer != NULL) {
> -*EndPointer = (CHAR16 *) String;
> -  }
> -  return RETURN_UNSUPPORTED;
> -}
> -
> -*Data = *Data * 10 + (*String - L'0');
> -String++;
> -  }
> -
> -  if (EndPointer != NULL) {
> -*EndPointer = (CHAR16 *) String;
> -  }
> -  return RETURN_SUCCESS;
> -}
> -
>  /**
>Convert a Null-terminated Unicode decimal string to a value of type UINT64.
>  
> @@ -1064,9 +998,9 @@ StrDecimalToUint64S (
>  
>  /**
>Convert a Null-terminated Unicode hexadecimal string to a value of type
> -  UINTN.
> +  UINT64.
>  
> -  This function outputs a value of type UINTN by interpreting the contents of
> +  This function outputs a value of type UINT64 by interpreting the contents 
> of
>the Unicode string specified by String as a hexadecimal number. The format 
> of
>the input Unicode string String is:
>  
> @@ -1091,8 +1025,8 @@ StrDecimalToUint64S (
>  
>If String has no valid hexadecimal digits in the above format, then 0 is
>stored at the location pointed to by Data.
> -  If the number represented by String exceeds the range defined by UINTN, 
> then
> -  MAX_UINTN is stored at the location pointed to by Data.
> +  If the number represented by String exceeds the range defined by UINT64, 
> then
> +  MAX_UINT64 is stored at the location pointed to by Data.
>  
>If 

Re: [edk2] [PATCH v2 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()

[Philippe Mathieu-Daudé]

On 30/11/18 23:45, Ard Biesheuvel wrote:
> Don't use the native word size string to number parsing routines,
> but instead, use the 64-bit one and cast to UINTN.
> 
> Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
> which takes care to use Strtoi64 () unless it assumes the value fits
> in 32-bit, so this change is a no-op even on 32-bit build hosts.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Jaben Carsey 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  BaseTools/Source/C/Common/CommonLib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> b/BaseTools/Source/C/Common/CommonLib.c
> index 7c559bc3c222..4a28f635f3a8 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -2252,9 +2252,9 @@ Strtoi (
>)
>  {
>if (IsHexStr (Str)) {
> -return StrHexToUintn (Str);
> +return (UINTN)StrHexToUint64 (Str);
>} else {
> -return StrDecimalToUintn (Str);
> +return (UINTN)StrDecimalToUint64 (Str);
>}
>  }
>  
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines

[Philippe Mathieu-Daudé]

On 30/11/18 23:45, Ard Biesheuvel wrote:
> Replace invocations of StrHexToUintn() with StrHexToUint64(), so
> that we can drop the former.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Jaben Carsey 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c 
> b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> index 555efa1acdde..6151926af9aa 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> @@ -520,7 +520,7 @@ EisaIdFromText (
>return (((Text[0] - 'A' + 1) & 0x1f) << 10)
> + (((Text[1] - 'A' + 1) & 0x1f) <<  5)
> + (((Text[2] - 'A' + 1) & 0x1f) <<  0)
> -   + (UINT32) (StrHexToUintn ([3]) << 16)
> +   + (UINT32) (StrHexToUint64 ([3]) << 16)
> ;
>  }
>  
> @@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
>  
>Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
>while (Index-- != 0) {
> -Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (, L'-'));
> +Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (, 
> L'-'));
>}
>  
>return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

[Philippe Mathieu-Daudé]

On 30/11/18 23:45, Ard Biesheuvel wrote:
> In the context of the BaseTools, there is no such thing as a native word
> size, given that the same set of tools may be used to build a firmware
> image consisting of both 32-bit and 64-bit modules.
> 
> So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> types instead of UINTN types when parsing strings.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  BaseTools/Source/C/Common/CommonLib.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> b/BaseTools/Source/C/Common/CommonLib.c
> index 618aadac781a..7c559bc3c222 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1785,7 +1785,7 @@ StrToIpv4Address (
>  {
>RETURN_STATUS  Status;
>UINTN  AddressIndex;
> -  UINTN  Uintn;
> +  UINT64 Uint64;
>EFI_IPv4_ADDRESS   LocalAddress;
>UINT8  LocalPrefixLength;
>CHAR16 *Pointer;
> @@ -1812,7 +1812,7 @@ StrToIpv4Address (
>  //
>  // Get D or P.
>  //
> -Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, , );
> +Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, , 
> );
>  if (RETURN_ERROR (Status)) {
>return RETURN_UNSUPPORTED;
>  }
> @@ -1820,18 +1820,18 @@ StrToIpv4Address (
>//
>// It's P.
>//
> -  if (Uintn > 32) {
> +  if (Uint64 > 32) {
>  return RETURN_UNSUPPORTED;
>}
> -  LocalPrefixLength = (UINT8) Uintn;
> +  LocalPrefixLength = (UINT8) Uint64;
>  } else {
>//
>// It's D.
>//
> -  if (Uintn > MAX_UINT8) {
> +  if (Uint64 > MAX_UINT8) {
>  return RETURN_UNSUPPORTED;
>}
> -  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> +  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
>AddressIndex++;
>  }
>  
> @@ -1888,7 +1888,7 @@ StrToIpv6Address (
>  {
>RETURN_STATUS  Status;
>UINTN  AddressIndex;
> -  UINTN  Uintn;
> +  UINT64 Uint64;
>EFI_IPv6_ADDRESS   LocalAddress;
>UINT8  LocalPrefixLength;
>CONST CHAR16   *Pointer;
> @@ -1969,7 +1969,7 @@ StrToIpv6Address (
>  //
>  // Get X.
>  //
> -Status = StrHexToUintnS (Pointer, , );
> +Status = StrHexToUint64S (Pointer, , );
>  if (RETURN_ERROR (Status) || End - Pointer > 4) {
>//
>// Number of hexadecimal digit characters is no more than 4.
> @@ -1978,24 +1978,24 @@ StrToIpv6Address (
>  }
>  Pointer = End;
>  //
> -// Uintn won't exceed MAX_UINT16 if number of hexadecimal digit 
> characters is no more than 4.
> +// Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit 
> characters is no more than 4.
>  //
>  ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
> -LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
> -LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
> +LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
> +LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
>  AddressIndex += 2;
>} else {
>  //
>  // Get P, then exit the loop.
>  //
> -Status = StrDecimalToUintnS (Pointer, , );
> -if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
> +Status = StrDecimalToUint64S (Pointer, , );
> +if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
>//
>// Prefix length should not exceed 128.
>//
>return RETURN_UNSUPPORTED;
>  }
> -LocalPrefixLength = (UINT8) Uintn;
> +LocalPrefixLength = (UINT8) Uint64;
>  Pointer = End;
>  break;
>}
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

[Udit Kumar]

Hi Laszlo 

> Are you using the latest edk2?
Yes, when I hit the bug, first step was to move to latest edk2. But this didn't 
helped.

I am not sure, why we are keeping PL011 Fifo mode off in default configuration. 
 

Regards
Udit

> -Original Message-
> From: Laszlo Ersek 
> Sent: Monday, December 3, 2018 3:27 PM
> To: Udit Kumar ; af...@apple.com
> Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Zeng, Star
> 
> Subject: Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot
> 
> On 11/30/18 10:13, Udit Kumar wrote:
> > Thanks Laszlo/Andrew
> > Finally I manage to get logs from user-space,  problem  was fifo of PL011 
> > uart
> was not getting enable in case of direct boot.
> > But in case of boot via UiApp, some piece of code was setting serial port
> attribute to enable this ( I still to figure out from where).
> > OS rely on boot-loader to enable this bit.
> 
> Are you using the latest edk2?
> 
> I vaguely recall some refactoring around PL011 (and in general, serial) 
> attributes
> from a year (or more?) ago. Hmm
> 
> b462f25a21e1 MdeModulePkg/SerialDxe: Describe correctly
> EFI_DEVICE_ERROR for SetAttributes
> 13d378fc82d4 MdeModulePkg/SerialDxe: Fix return valued in SerialSetAttributes
> 7ce5af40c98b MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is
> not supported
> 
> and
> 
> 91cc526b15ff MdeModulePkg/SerialDxe: Fix not able to change serial attributes
> 
> Thanks
> Laszlo
> 
> >> -Original Message-
> >> From: Laszlo Ersek 
> >> Sent: Thursday, November 29, 2018 11:31 PM
> >> To: Udit Kumar ; af...@apple.com
> >> Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Zeng, Star
> >> 
> >> Subject: Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct
> boot
> >>
> >> On 11/29/18 14:12, Udit Kumar wrote:
> >>> Thanks Laszlo,
> >>>
> >>>
>  I can only think of some terminal control sequences that are *not*
>  printed to the terminal when you don't enter UiApp manually. I don't
>  understand how that could cause the exact symptom you describe, but I
> have
> >> no better explanation.
> 
>  Can you try other serial communication programs on your desktop? Such
>  as "minicom" or "screen"?
> >>>
> >>> Screen didn't help.
> >>> Moreover , using different OS distributions show same similar behavior !!
> >>>
>  Also, can you try changing your "console=..." kernel param(s)?
> >>>
> >>> You meant baud-rate ?
> >>
> >> Yes, and more. The options that the "console=" kernel parameter takes.
> >>
> >>>
> >>> On uefi side,  could you help me if there is some extra information
> >>> passed to OS in path UiApp -> BootDevice,
> >>
> >> I don't think so. Nothing comes to my mind anyway.
> >>
> >>> I could see , some of additional protocols are installed in above
> >>> path, I am not sure if those are used  by  OS or OS Loader (grub in my 
> >>> case)
> >> somehow.
> >>
> >> Well, UiApp generally connects all drivers to all devices -- normally a 
> >> platform
> >> BDS would not want to do this, for the sake of booting quickly --, which 
> >> likely
> >> results in more protocol instances being installed in the system. That
> shouldn't
> >> cause a difference for how serial behaves once the OS has booted.
> >>
> >> Thanks
> >> Laszlo

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

Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

[Laszlo Ersek]

On 11/30/18 10:13, Udit Kumar wrote:
> Thanks Laszlo/Andrew
> Finally I manage to get logs from user-space,  problem  was fifo of PL011 
> uart was not getting enable in case of direct boot.
> But in case of boot via UiApp, some piece of code was setting serial port 
> attribute to enable this ( I still to figure out from where). 
> OS rely on boot-loader to enable this bit.

Are you using the latest edk2?

I vaguely recall some refactoring around PL011 (and in general, serial) 
attributes from a year (or more?) ago. Hmm

b462f25a21e1 MdeModulePkg/SerialDxe: Describe correctly EFI_DEVICE_ERROR for 
SetAttributes
13d378fc82d4 MdeModulePkg/SerialDxe: Fix return valued in SerialSetAttributes
7ce5af40c98b MdeModulePkg/SerialDxe: Do not fail reset when SetAttributes is 
not supported

and

91cc526b15ff MdeModulePkg/SerialDxe: Fix not able to change serial attributes

Thanks
Laszlo

>> -Original Message-
>> From: Laszlo Ersek 
>> Sent: Thursday, November 29, 2018 11:31 PM
>> To: Udit Kumar ; af...@apple.com
>> Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Zeng, Star
>> 
>> Subject: Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot
>>
>> On 11/29/18 14:12, Udit Kumar wrote:
>>> Thanks Laszlo,
>>>
>>>
 I can only think of some terminal control sequences that are *not*
 printed to the terminal when you don't enter UiApp manually. I don't
 understand how that could cause the exact symptom you describe, but I have
>> no better explanation.

 Can you try other serial communication programs on your desktop? Such
 as "minicom" or "screen"?
>>>
>>> Screen didn't help.
>>> Moreover , using different OS distributions show same similar behavior !!
>>>
 Also, can you try changing your "console=..." kernel param(s)?
>>>
>>> You meant baud-rate ?
>>
>> Yes, and more. The options that the "console=" kernel parameter takes.
>>
>>>
>>> On uefi side,  could you help me if there is some extra information
>>> passed to OS in path UiApp -> BootDevice,
>>
>> I don't think so. Nothing comes to my mind anyway.
>>
>>> I could see , some of additional protocols are installed in above
>>> path, I am not sure if those are used  by  OS or OS Loader (grub in my case)
>> somehow.
>>
>> Well, UiApp generally connects all drivers to all devices -- normally a 
>> platform
>> BDS would not want to do this, for the sake of booting quickly --, which 
>> likely
>> results in more protocol instances being installed in the system. That 
>> shouldn't
>> cause a difference for how serial behaves once the OS has booted.
>>
>> Thanks
>> Laszlo

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

Re: [edk2] [PATCH edk2-platforms 09/27] Silicon/NXP: Add headers for SoC-specific i.MX packages to use

[Leif Lindholm]

On Sat, Dec 01, 2018 at 12:22:17AM +, Chris Co wrote:
> > If using EFI_ACPI prefix, these #defines really should be in edk2 MdePkg. 
> > And
> > CSRT itself is, so that might not be a bad idea.
> > 
> > > +
> > > +#pragma pack(push, 1)
> > 
> > I don't see this #pragma making any difference to the structs below, can it 
> > be
> > dropped?
> 
> The pragma pack is defensive. Without it, we rely on the compiler
> packing structures by default and this may not happen on 64 bit
> compiles.

I understand that is what the pragma does. My comment was because all
of the variables in the below structs are naturally aligned.
The reason I dislike its use when effectively a no-op, is that it
makes it look like it it isn't a no-op.

If it covers a larger set of structs, some of which require the packed
attribute I'm OK with that. But I'm not a fan of adding it "just in
case" without contemplating the statement's (lack of) effect.

Regards,

Leif

> I have addressed the remaining feedback and will resubmit with v2.
> 
> Thanks,
> Chris
> 
> > > +//---
> > > +- // CSRT Resource Group header 24 bytes long
> > > +//---
> > > +-
> > > +typedef struct {
> > > +  UINT32 Length;
> > > +  UINT32 VendorID;
> > > +  UINT32 SubVendorId;
> > > +  UINT16 DeviceId;
> > > +  UINT16 SubdeviceId;
> > > +  UINT16 Revision;
> > > +  UINT16 Reserved;
> > > +  UINT32 SharedInfoLength;
> > > +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> > > +
> > > +//---
> > > +- // CSRT Resource Descriptor 12 bytes total
> > > +//---
> > > +-
> > > +typedef struct {
> > > +  UINT32 Length;
> > > +  UINT16 ResourceType;
> > > +  UINT16 ResourceSubType;
> > > +  UINT32 UID;
> > > +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
> > > +#pragma pack (pop)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [edk2-announce] Research Request

[Laszlo Ersek]

On 11/29/18 22:20, Rebecca Cran wrote:
> Would you be interested in going through this process with Phabricator, too? 

Sure! Just tell me where to create an account.

Thanks,
Laszlo

> On November 29, 2018 at 2:48:18 AM, Laszlo Ersek 
> (ler...@redhat.com(mailto:ler...@redhat.com)) wrote:
> 
>> On 11/29/18 02:07, Jeremiah Cox wrote:
>>> I did a further experiment for you:
>>> https://github.com/lersek/edk2/pull/2
>>
>> Thanks!
>>
>>> I cannot rebase away my history from PRs...
>>> Hopefully you have a nice email trail too.
>>
>> The emails are coming in nice, but I'm not universally pleased with the
>> contents. I listed some issues regarding that in
>> , but I guess I should write them
>> up sometime more readably, at the end of this experiment.
>>
>> Thanks!
>> Laszlo
>>
>>> -Original Message-
>>> From: Laszlo Ersek 
>>> Sent: Wednesday, November 28, 2018 2:02 PM
>>> To: Jeremiah Cox ; Brian J. Johnson 
>>> ; stephano 
>>> Cc: edk2-devel@lists.01.org
>>> Subject: Re: [edk2] [edk2-announce] Research Request
>>>
>>> On 11/28/18 19:31, Jeremiah Cox wrote:
 Test PR submitted
>>>
>>> Thanks!
>>> Laszlo
>>>
>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

[edk2] [edk2-test][v2 Patch 2/3] uefi-sct/SctPkg:Add VerifySignature() Func Test

[Eric Jin]

Enable the BBTestVerifySignatureFunctionTest()
with 2 checkpoints below, it should be success.
The Certificate/Hash/Digest/signedData are updated
correspondingly.
a)Signed hash was verified against caller-provided
hash of content, the signer's certificate was not
found in RevokedDb, and was found in AllowedDb.
b)Signer is found in both AllowedDb and RevokedDb,
the signing was allowed by reference to TimeStampDb,
and no hash matching content hash was found in RevokedDb.

Cc: Supreeth Venkatesh 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin 
---
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |2 +
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |8 +
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c | 1466 +---
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestFunction.c |   86 ++
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |   57 +-
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.h |   25 +-
 6 files changed, 1099 insertions(+), 545 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
index 142f6d4..4d433c3 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
@@ -42,3 +42,5 @@ EFI_GUID gPkcs7BBTestFunctionAssertionGuid001 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASS
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid002 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_002_GUID;
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid003 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_003_GUID;
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid004 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_004_GUID;
+EFI_GUID gPkcs7BBTestFunctionAssertionGuid005 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_005_GUID;
+EFI_GUID gPkcs7BBTestFunctionAssertionGuid006 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_006_GUID;
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
index ce980c9..94d2568 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
@@ -81,3 +81,11 @@ extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid003;
 #define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_004_GUID \
 { 0x912e23ef, 0x299c, 0x41ab, {0xa0, 0xf5, 0xfc, 0xbc, 0xf6, 0xfd, 0xd3, 0x32 
}}
 extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid004;
+
+#define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_005_GUID \
+{ 0x93740b06, 0xa186, 0x47ff, { 0xba, 0xc3, 0xdd, 0xa8, 0xcb, 0x7b, 0x18, 0x5e 
}}
+extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid005;
+
+#define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_006_GUID \
+{ 0x37253616, 0xca42, 0x4082, { 0x90, 0xda, 0xdb, 0x69, 0x98, 0x22, 0xa0, 0xe6 
}}
+extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid006;
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
index 0511e00..9b66938 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
@@ -25,541 +25,979 @@ Abstract:
 --*/
 
 //
+// Test Root Certificate ("TestRoot.cer")
+//
+GLOBAL_REMOVE_IF_UNREFERENCED UINT8 TestRootCert[781] = {
+  0x30, 0x82, 0x03, 0x09, 0x30, 0x82, 0x01, 0xF1, 0xA0, 0x03, 0x02, 0x01,
+  0x02, 0x02, 0x10, 0xDE, 0x9F, 0x42, 0x91, 0x68, 0x16, 0xEA, 0x97, 0x4D,
+  0xA1, 0x8A, 0x32, 0x25, 0xD6, 0xEE, 0x8D, 0x30, 0x0D, 0x06, 0x09, 0x2A,
+  0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x0B, 0x05, 0x00, 0x30, 0x13,
+  0x31, 0x11, 0x30, 0x0F, 0x06, 0x03, 0x55, 0x04, 0x03, 0x13, 0x08, 0x54,
+  0x65, 0x73, 0x74, 0x52, 0x6F, 0x6F, 0x74, 0x30, 0x1E, 0x17, 0x0D, 0x31,
+  0x38, 0x30, 0x31, 0x32, 0x35, 0x30, 0x32, 0x30, 0x35, 0x35, 0x30, 0x5A,
+  0x17, 0x0D, 0x33, 0x39, 0x31, 0x32, 0x33, 0x31, 0x32, 0x33, 0x35, 0x39,
+  0x35, 0x39, 0x5A, 0x30, 0x13, 0x31, 0x11, 0x30, 0x0F, 0x06, 0x03, 0x55,
+  0x04, 0x03, 0x13, 0x08, 0x54, 0x65, 0x73, 0x74, 0x52, 0x6F, 0x6F, 0x74,
+  0x30, 0x82, 0x01, 0x22, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86,
+  0xF7, 0x0D, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, 0x0F, 0x00,
+  0x30, 0x82, 0x01, 0x0A, 0x02, 0x82, 0x01, 0x01, 0x00, 0xA5, 0x97, 0x23,
+  0x48, 0xBE, 0xCA, 0xC8, 0xE0, 0x88, 0xC6, 0xA2, 0xAF, 0x78, 0x60, 0x94,
+  0x48, 0x3E, 0x82, 0xE7, 0xD5, 0x62, 0x01, 0x73, 0x00, 0xEA, 0x42, 0x7A,
+  0x32, 0x0A, 0xD7, 0x3F, 0x4D, 0x0B, 0x71, 0x6D, 0xD3, 0x50, 0x5E, 0x26,
+  0x20, 0xE8, 0xCC, 0xB6, 0x0A, 0xAF, 0xD9, 0x07, 0x22, 0x17, 0x45, 0xD8,
+  0x91, 0x75, 0x75, 0x52, 0xD8, 0x8C, 0xAB, 0x63, 0x0A, 0xF0, 0x23, 0x14,
+  0x34, 0x92, 0x3F, 0xE0, 0x05, 0x24, 0x28, 0xED, 0x74, 0x8E, 0x4D, 0x3E,
+  

[edk2] [edk2-test][v2 Patch 0/3] Add VerifySignature() Test

[Eric Jin]

This is the cover letter.
The series of patches are listed below.

  uefi-sct/SctPkg:Format code style in PKCS7Verify
  uefi-sct/SctPkg:Add VerifySignature() Func Test
  uefi-sct/SctPkg:Add VerifySignature() Conf Test


 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |   93 +-
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |  192 +--
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTest.inf   |  126 +-
 .../BlackBoxTest/Pkcs7BBTestConformance.c  | 1305 +---
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c | 1568 +---
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestFunction.c |  553 ---
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |  458 +++---
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.h |  248 ++--
 8 files changed, 2689 insertions(+), 1854 deletions(-)

-- 
2.9.0.windows.1

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

[edk2] [PATCH 5/5] StandaloneMmPkg: Update dependency on PeCoffExtraActionLib

[Sughosh Ganu]

From: Achin Gupta 

Replace DebugPeCoffExtraActionLib with StandaloneMmExtraActionLib

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Sughosh Ganu 
---
 StandaloneMmPkg/StandaloneMmPkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc 
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index 412702272c95..ea4153f567a8 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -65,7 +65,7 @@ [LibraryClasses.AARCH64]
   
StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
   ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
   
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
-  
PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
+  
PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
   
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   # ARM PL011 UART Driver
-- 
2.7.4

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

[edk2] [edk2-test][v2 Patch 3/3] uefi-sct/SctPkg:Add VerifySignature() Conf Test

[Eric Jin]

Enable the BBTestVerifySignatureConformanceTest()
with checkpoints under 7 cases below, it should
not be success.
a)Signature is NULL/SignatureSize is 0/InHash is NULL
/InHashSize is 0/AllowedDb is NULL
b)Modify the Data in P7TestSignature to make it invalid
c)Modify the Data in InHash to make it invalid
d)Input the invalid Hashsize
e)The SignedData buffer was correctly formatted but
signer was in RevokedDb
f)The SignedData buffer was correctly formatted but
signer was not in AllowedDb
g)Matching content hash found in the RevokedDb

Cc: Supreeth Venkatesh 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin 
---
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |   5 +-
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |  16 ++
 .../BlackBoxTest/Pkcs7BBTestConformance.c  | 301 +
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |   9 +
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.h |   8 +
 5 files changed, 338 insertions(+), 1 deletion(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
index 4d433c3..8f6546a 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
@@ -36,7 +36,10 @@ EFI_GUID gPkcs7BBTestConformanceAssertionGuid007 = 
EFI_TEST_PKCS7BBTESTCONFORMAN
 EFI_GUID gPkcs7BBTestConformanceAssertionGuid008 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_008_GUID;
 EFI_GUID gPkcs7BBTestConformanceAssertionGuid009 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_009_GUID;
 EFI_GUID gPkcs7BBTestConformanceAssertionGuid010 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_010_GUID;
-
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid011 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_011_GUID;
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid012 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_012_GUID;
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid013 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_013_GUID;
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid014 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_014_GUID;
 
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid001 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_001_GUID;
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid002 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_002_GUID;
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
index 94d2568..1827207 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
@@ -65,6 +65,22 @@ extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid009;
 { 0xb136e016, 0x4f80, 0x44bd, {0xba, 0xb0, 0x1c, 0x34, 0x8a, 0x2d, 0xa1, 0x8a 
}}
 extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid010;
 
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_011_GUID \
+{ 0xa58f3626, 0xf16e, 0x4cd5, { 0xba, 0x12, 0x7a, 0x9d, 0xd6, 0x9a, 0x7a, 0x71 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid011;
+
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_012_GUID \
+{ 0xbe4a0bf2, 0x2d46, 0x4d96, { 0xa6, 0x11, 0x21, 0x99, 0xa5, 0x5f, 0xa4, 0xee 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid012;
+
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_013_GUID \
+{ 0xc0536a27, 0x304e, 0x497a, { 0xa5, 0xe3, 0x94, 0x11, 0x38, 0x53, 0x6e, 0x40 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid013;
+
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_014_GUID \
+{ 0x8c5aa0e8, 0x17ff, 0x49cd, { 0x8f, 0xec, 0x37, 0xc3, 0xfd, 0x5f, 0x77, 0x0 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid014;
+
 
 #define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_001_GUID \
 { 0x5c0eec50, 0xa6ea, 0x413c, {0x8a, 0x46, 0x4a, 0xd1, 0x4a, 0x77, 0x76, 0xf1 
}}
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
index 1dc9921..ce7d5bb 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
@@ -502,3 +502,304 @@ BBTestVerifyBufferConformanceTest (
 
   return EFI_SUCCESS;
 }
+
+EFI_STATUS
+BBTestVerifySignatureConformanceTest (
+  IN EFI_BB_TEST_PROTOCOL*This,
+  IN VOID*ClientInterface,
+  IN EFI_TEST_LEVEL  TestLevel,
+  IN EFI_HANDLE  SupportHandle
+  )
+{
+  EFI_STANDARD_TEST_LIBRARY_PROTOCOL*StandardLib;
+  EFI_STATUSStatus;
+  EFI_PKCS7_VERIFY_PROTOCOL *Pkcs7Verify;
+  EFI_TEST_ASSERTIONAssertionType;
+
+  Pkcs7Verify = (EFI_PKCS7_VERIFY_PROTOCOL*)ClientInterface;
+  

[edk2] [PATCH 4/5] StandaloneMmPkg: Replace dependency on ArmMmuLib

[Sughosh Ganu]

From: Achin Gupta 

Use StandaloneMmMmuLib instead of ArmMmuLib in StandaloneMmPkg for AArch64

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Sughosh Ganu 
---
 StandaloneMmPkg/StandaloneMmPkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc 
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index 3470e1102cc3..412702272c95 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -62,7 +62,7 @@ [LibraryClasses]
 
 [LibraryClasses.AARCH64]
   ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
-  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuStandaloneMmCoreLib.inf
+  
StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
   ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
   
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
   
PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
-- 
2.7.4

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

[edk2] [PATCH 3/5] StandaloneMmPkg: Zero data structure explicitly

[Sughosh Ganu]

From: Achin Gupta 

Introduction of the -mstrict-align flag results in GCC attempting
to use memset to zero out the InitMmFoundationSvcArgs structure.
In the absence of this C library function, this patch explicitly
zeroes this data structure prior to use.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Sughosh Ganu 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 20fd0d1f34ba..05ed6c8dd0b5 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -224,7 +224,7 @@ _ModuleEntryPoint (
 {
   PE_COFF_LOADER_IMAGE_CONTEXTImageContext;
   EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo;
-  ARM_SVC_ARGSInitMmFoundationSvcArgs = {0};
+  ARM_SVC_ARGSInitMmFoundationSvcArgs;
   EFI_STATUS  Status;
   UINT32  SectionHeaderOffset;
   UINT16  NumberOfSections;
@@ -299,6 +299,7 @@ _ModuleEntryPoint (
   DEBUG ((DEBUG_INFO, "Shared Cpu Driver EP 0x%lx\n", (UINT64) 
CpuDriverEntryPoint));
 
 finish:
+  ZeroMem (, sizeof(InitMmFoundationSvcArgs));
   InitMmFoundationSvcArgs.Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
   InitMmFoundationSvcArgs.Arg1 = Status;
   DelegatedEventLoop ();
-- 
2.7.4

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

[edk2] [PATCH 2/5] StandaloneMmPkg: Enforce alignment check for AArch64

[Sughosh Ganu]

From: Achin Gupta 

On AArch64, Standalone MM during the SEC phase runs in S-EL0 with
SCTLR_EL1.A=1. This patch adds the -mstrict-align compiler flag to
ensure that the generated code is compliant with the runtime
alignment checks.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Sughosh Ganu 
---
 StandaloneMmPkg/StandaloneMmPkg.dsc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc 
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index 84de1ab0f13a..3470e1102cc3 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -128,4 +128,5 @@ [Components.AARCH64]
 #
 
###
 [BuildOptions.AARCH64]
-GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -march=armv8-a+nofp
+GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -march=armv8-a+nofp 
-mstrict-align
+GCC:*_*_*_CC_FLAGS = -mstrict-align
-- 
2.7.4

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

[edk2] [PATCH 1/5] StandaloneMmPkg: Add missing dependency on PL011UartClockLib

[Sughosh Ganu]

From: Achin Gupta 

This patch fixes the dependency PL011UartLib has on PL011UartClockLib by
including its implementation path in the StandaloneMm DSC file.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Achin Gupta 
Signed-off-by: Sughosh Ganu 
---
 StandaloneMmPkg/StandaloneMmPkg.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc 
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index c6ddb35b8993..84de1ab0f13a 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -67,6 +67,7 @@ [LibraryClasses.AARCH64]
   
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
   
PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
   PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
+  
PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
   # ARM PL011 UART Driver
   
SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
 
-- 
2.7.4

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