Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf handler

2020-06-28 Thread Gao, Zhichao


> -Original Message-
> From: Ni, Ray 
> Sent: Monday, June 29, 2020 10:42 AM
> To: Gao, Zhichao ; Laszlo Ersek ;
> devel@edk2.groups.io
> Cc: Wu, Hao A ; Wang, Jian J ;
> Gao, Liming 
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf
> handler
> 
> 
> 
> > -Original Message-
> > From: Gao, Zhichao 
> > Sent: Monday, June 29, 2020 9:47 AM
> > To: Laszlo Ersek ; Ni, Ray ;
> > devel@edk2.groups.io
> > Cc: Wu, Hao A ; Wang, Jian J
> > ; Gao, Liming 
> > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate
> > the Udf handler
> >
> > Hi Laszlo,
> >
> > Sorry, I didn't put the detail info about the issue. Let me descript here.
> >
> > The issue is not only for Red Hat. I found it with ubuntu 18.02 amd64
> > and
> > Fedora-20-x86_64 ISO image as well. I didn't view all the linux iso images.
> >
> > Here is the issue:
> > Using USB CD ROM with the linux ISO DVD. Run the platform and enter
> > UEFI shell -> plug the USB CD ROM with the ISO DVD -> run "map -r".
> > The CD's file system didn't show.
> >
> > It works fine with the normal boot (such as F2, F7 and auto boot)
> > because it would run the *connect all* operation. That would run the
> > partition driver serval times. First time the partition driver would
> > install MBR partition info protocol with the device handle and skip
> > the UDF check. The second time, it would fail the MBR check with
> > *already started* and run UDF check next. That means for such ISO the
> > UEFI would install two partition protocol, both MBR and UDF.
> Zhichao,
> It sounds like a bug in partition driver that the second UDF check succeeds.

I agree with you. It is a bug when return *already start* and continue to check 
next partition format. That is why the issue is not observed when the add UDF 
support.

Thanks,
Zhichao

> 
> 
> >
> > But for shell environment, the USB hot plug handle would connect the
> > USB device only once and missing the UDF check.
> >
> > I don't know why linux need the MBR info. Maybe for legacy compatible
> > thinking (my opinion, may be wrong).
> >
> > If the GPT, MBR and UDF are conflict, then the original logic is fine.
> > But in fact, GPT and MBR are conflict and GPT/MBR and UDF are not conflict.
> >
> > Thanks,
> > Zhichao
> >
> > > -Original Message-
> > > From: Laszlo Ersek 
> > > Sent: Thursday, June 25, 2020 5:02 PM
> > > To: Ni, Ray ; devel@edk2.groups.io; Gao, Zhichao
> > > 
> > > Cc: Wu, Hao A ; Wang, Jian J
> > > ; Gao, Liming 
> > > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe:
> > > Seperate the
> > Udf
> > > handler
> > >
> > > On 06/25/20 02:21, Ni, Ray wrote:
> > > > Laszlo,
> > > > You are surprised because the issue is related to Redhat, or
> > > > because the word "Redhat" should not be mentioned in the Bugzilla?
> > >
> > > It's totally fine to mention Red Hat (with correct spelling) in the
> > > issue
> > description.
> > > The more details, the better.
> > >
> > > I'm surprised that the issue *seems* specific to Red Hat. Earlier I
> > > had looked
> > on-
> > > and-off on the scripts that generate (for example) the Fedora ISO
> > > images (so
> > that
> > > they be both BIOS-bootable and UEFI-bootable), and I don't recall
> > > any
> > particular
> > > tricks that would exploit gray areas in specs or so.
> > >
> > > It's possible that the *tools* the scripts use for formatting the
> > > ISOs have some bugs or quirks, of course. (But then I'm again a bit
> > > surprised for those issues
> > or
> > > quirks to be RH-specific -- I just don't see a reason for us to
> > > diverge from the respective upstream code bases of those tools.)
> > >
> > > > http://manpages.ubuntu.com/manpages/bionic/man8/mkudffs.8.html
> > > > is a tool that creates UDF file system with a compatible MBR table
> > > > in the top 32KB specially for Windows.
> > > >
> > > > Search " WHOLE DISK VS PARTITION" in the web page.
> > >
> > > Thanks -- and, indeed, this confirms my point. The issue (or
> > > unexpected
> > behavior
> > > at least) is not specific to Red Hat, it is part of the (most
> > > likely: upstream) mkudffs tool. (Case in point, the manual page
> > > you're quoting
> > is
> > > from an Ubuntu distro.)
> > >
> > > I think a minimal reproducer using the mkudffs command (and maybe
> > > more
> > > commands) would be nice in the TianoCore BZ.
> > >
> > > Anyway, looking at the manual you've identified, it does seem like a
> > > gray
> > area.
> > > It's a conflict between two specifications, apparently. Or perhaps
> > > more
> > precisely,
> > > a bug in the UDF spec. (It seems really strange that a filesystem
> > > insists on
> > being
> > > installed on a complete block device, and not on a partition
> > > described in a standard partition table.)
> > >
> > > The workaround described in the manual is presented as a workaround
> > > "for Windows' sake". I don't think that's fair; in my (uneducated?)
> > > opinion
> > > *any* file system should be happy to 

Re: [edk2-devel] [RFC edk2 v1 0/1] Fix a infrequent issue in variable

2020-06-28 Thread Ming Huang via groups.io
Hi Guomin,

Ok, I will send patch out today.

Thanks,
Ming

在 2020/6/29 11:03, Jiang, Guomin 写道:
> Hi Huang,
> 
> Could you send the normal patch rather than RFC?
> 
> Best Regards
> Guomin
> 
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of Ming
>> Huang
>> Sent: Monday, May 25, 2020 7:34 PM
>> To: devel@edk2.groups.io; Wang, Jian J ; Wu, Hao A
>> ; Gao, Liming 
>> Cc: lidongz...@huawei.com; huangmin...@huawei.com;
>> songdongku...@huawei.com; wanghuiqi...@huawei.com;
>> qiulian...@huawei.com; shenli...@huawei.com
>> Subject: [edk2-devel] [RFC edk2 v1 0/1] Fix a infrequent issue in variable
>>
>> https://bugzilla.tianocore.org/show_bug.cgi?id=2667
>>
>> Ming Huang (1):
>>   MdeModulePkg/Variable: Move FindVariable after
>> AutoUpdateLangVariable
>>
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 26
>> ++--
>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> --
>> 2.8.1
>>
>>
>> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61783): https://edk2.groups.io/g/devel/message/61783
Mute This Topic: https://groups.io/mt/74462884/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH] FmpDevicePkg: Enhance capsule verification with secure boot keys

2020-06-28 Thread Guomin Jiang
I think it have some vulnerability, the case as below.

1. Untrusted End User enroll the new DB key -> sign the untrusted device 
firmware -> flash the untrusted device firmware -> the system will become 
unsafe.

I think the end user is untrusted and we need to make sure only few person can 
have the privilege.

Best Regards
Guomin

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Liming
> Sun
> Sent: Saturday, June 20, 2020 1:48 AM
> To: Xu, Wei6 ; Gao, Liming ;
> Kinney, Michael D 
> Cc: Liming Sun ; devel@edk2.groups.io; Sean Brogan
> 
> Subject: [edk2-devel] [PATCH] FmpDevicePkg: Enhance capsule verification
> with secure boot keys
> 
> This commit enhances the FmpDevicePkg package to optionally verify
> capsule with the secure boot keys when PcdFmpDevicePkcs7CertBufferXdr is
> not set and the new PCD variable PcdFmpDeviceAllowSecureBootKeys is
> configured. Below is the check logic:
>   - Pass if verified with PK key, or PK key not set yet;
>   - Deny if verified with the DBX keys;
>   - Verified it against the DB keys;
> 
> One purpose for this change is to auto-deploy the UEFI secure boot keys
> with UEFI capsule. Initially it's done in trusted environment.
> Once secure boot is enabled, the same keys will be used to verify the signed
> capsules as well for further updates.
> 
> Signed-off-by: Liming Sun 
> ---
>  FmpDevicePkg/FmpDevicePkg.dec |   6 +++
>  FmpDevicePkg/FmpDxe/FmpDxe.c  | 109
> --
>  FmpDevicePkg/FmpDxe/FmpDxe.h  |   1 +
>  FmpDevicePkg/FmpDxe/FmpDxe.inf|   3 ++
>  FmpDevicePkg/FmpDxe/FmpDxeLib.inf |   1 +
>  5 files changed, 117 insertions(+), 3 deletions(-)
> 
> diff --git a/FmpDevicePkg/FmpDevicePkg.dec
> b/FmpDevicePkg/FmpDevicePkg.dec index cab63f5..3aeb89c 100644
> --- a/FmpDevicePkg/FmpDevicePkg.dec
> +++ b/FmpDevicePkg/FmpDevicePkg.dec
> @@ -126,6 +126,12 @@
># @Prompt Firmware Device Image Type ID
> 
> gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceImageTypeIdGuid|{0}|VOID
> *|0x4010
> 
> +  ## This option is used to verify the capsule using secure boot keys
> + if the  # PcdFmpDevicePkcs7CertBufferXdr is not configured. In such
> + case, the check  # will pass if secure boot hasn't been enabled yet.
> +  # @A flag to tell whether to use secure boot keys when
> PcdFmpDevicePkcs7CertBufferXdr is not set.
> +
> +
> gFmpDevicePkgTokenSpaceGuid.PcdFmpDeviceAllowSecureBootKeys|0x0|
> UINT8|
> + 0x4012
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
>## One or more PKCS7 certificates used to verify a firmware device capsule
>#  update image.  Encoded using the Variable-Length Opaque Data format
> of RFC diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c index 5884177..6f82aee 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -682,6 +682,102 @@ GetAllHeaderSize (
>return CalculatedSize;
>  }
> 
> +EFI_STATUS
> +CheckTheImageWithSecureBootVariable (
> +  IN CONST CHAR16*Name,
> +  IN CONST EFI_GUID  *Guid,
> +  IN CONST VOID  *Image,
> +  IN UINTN   ImageSize
> +  )
> +{
> +  EFI_STATUS  Status;
> +  VOID*Data;
> +  UINTN   Length;
> +  EFI_SIGNATURE_LIST  *CertList;
> +  EFI_SIGNATURE_DATA  *CertData;
> +  UINTN   CertCount;
> +  UINTN   Index;
> +
> +  Status = GetVariable2 (Name, Guid, , );  if (EFI_ERROR
> + (Status)) {
> +return EFI_NOT_FOUND;
> +  }
> +
> +  CertList = (EFI_SIGNATURE_LIST *) Data;  while ((Length > 0) &&
> + (Length >= CertList->SignatureListSize)) {
> +if (CompareGuid (>SignatureType, )) {
> +  CertData  = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList +
> +sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);
> +  CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) 
> -
> +CertList->SignatureHeaderSize) / CertList->SignatureSize;
> +
> +  for (Index = 0; Index < CertCount; Index++) {
> +Status = AuthenticateFmpImage (
> +   (EFI_FIRMWARE_IMAGE_AUTHENTICATION *)Image,
> +   ImageSize,
> +   CertData->SignatureData,
> +   CertList->SignatureSize - sizeof (EFI_GUID)
> +   );
> +if (!EFI_ERROR (Status))
> +  goto Done;
> +
> +CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertList-
> >SignatureSize);
> +  }
> +}
> +
> +Length -= CertList->SignatureListSize;
> +CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList +
> + CertList->SignatureListSize);  }
> +
> +Done:
> +  FreePool (Data);
> +  return Status;
> +}
> +
> +EFI_STATUS
> +CheckTheImageWithSecureBootKeys (
> +  IN  CONST VOID  *Image,
> +  IN  UINTN   ImageSize
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  // PK check.
> +  Status = CheckTheImageWithSecureBootVariable(
> + EFI_PLATFORM_KEY_NAME,
> + ,
> + Image,
> +

Re: [edk2-devel] [RFC edk2 v1 0/1] Fix a infrequent issue in variable

2020-06-28 Thread Guomin Jiang
Hi Huang,

Could you send the normal patch rather than RFC?

Best Regards
Guomin

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ming
> Huang
> Sent: Monday, May 25, 2020 7:34 PM
> To: devel@edk2.groups.io; Wang, Jian J ; Wu, Hao A
> ; Gao, Liming 
> Cc: lidongz...@huawei.com; huangmin...@huawei.com;
> songdongku...@huawei.com; wanghuiqi...@huawei.com;
> qiulian...@huawei.com; shenli...@huawei.com
> Subject: [edk2-devel] [RFC edk2 v1 0/1] Fix a infrequent issue in variable
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2667
> 
> Ming Huang (1):
>   MdeModulePkg/Variable: Move FindVariable after
> AutoUpdateLangVariable
> 
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 26
> ++--
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> --
> 2.8.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61781): https://edk2.groups.io/g/devel/message/61781
Mute This Topic: https://groups.io/mt/74462884/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf handler

2020-06-28 Thread Ni, Ray


> -Original Message-
> From: Gao, Zhichao 
> Sent: Monday, June 29, 2020 9:47 AM
> To: Laszlo Ersek ; Ni, Ray ;
> devel@edk2.groups.io
> Cc: Wu, Hao A ; Wang, Jian J ;
> Gao, Liming 
> Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the
> Udf handler
> 
> Hi Laszlo,
> 
> Sorry, I didn't put the detail info about the issue. Let me descript here.
> 
> The issue is not only for Red Hat. I found it with ubuntu 18.02 amd64 and
> Fedora-20-x86_64 ISO image as well. I didn't view all the linux iso images.
> 
> Here is the issue:
> Using USB CD ROM with the linux ISO DVD. Run the platform and enter UEFI
> shell -> plug the USB CD ROM with the ISO DVD -> run "map -r". The CD's file
> system didn't show.
> 
> It works fine with the normal boot (such as F2, F7 and auto boot) because it
> would run the *connect all* operation. That would run the partition driver
> serval times. First time the partition driver would install MBR partition info
> protocol with the device handle and skip the UDF check. The second time, it
> would fail the MBR check with *already started* and run UDF check next. That
> means for such ISO the UEFI would install two partition protocol, both MBR and
> UDF.
Zhichao,
It sounds like a bug in partition driver that the second UDF check succeeds.


> 
> But for shell environment, the USB hot plug handle would connect the USB
> device only once and missing the UDF check.
> 
> I don't know why linux need the MBR info. Maybe for legacy compatible
> thinking (my opinion, may be wrong).
> 
> If the GPT, MBR and UDF are conflict, then the original logic is fine. But in 
> fact,
> GPT and MBR are conflict and GPT/MBR and UDF are not conflict.
> 
> Thanks,
> Zhichao
> 
> > -Original Message-
> > From: Laszlo Ersek 
> > Sent: Thursday, June 25, 2020 5:02 PM
> > To: Ni, Ray ; devel@edk2.groups.io; Gao, Zhichao
> > 
> > Cc: Wu, Hao A ; Wang, Jian J ;
> > Gao, Liming 
> > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the
> Udf
> > handler
> >
> > On 06/25/20 02:21, Ni, Ray wrote:
> > > Laszlo,
> > > You are surprised because the issue is related to Redhat, or because
> > > the word "Redhat" should not be mentioned in the Bugzilla?
> >
> > It's totally fine to mention Red Hat (with correct spelling) in the issue
> description.
> > The more details, the better.
> >
> > I'm surprised that the issue *seems* specific to Red Hat. Earlier I had 
> > looked
> on-
> > and-off on the scripts that generate (for example) the Fedora ISO images (so
> that
> > they be both BIOS-bootable and UEFI-bootable), and I don't recall any
> particular
> > tricks that would exploit gray areas in specs or so.
> >
> > It's possible that the *tools* the scripts use for formatting the ISOs have 
> > some
> > bugs or quirks, of course. (But then I'm again a bit surprised for those 
> > issues
> or
> > quirks to be RH-specific -- I just don't see a reason for us to diverge 
> > from the
> > respective upstream code bases of those tools.)
> >
> > > http://manpages.ubuntu.com/manpages/bionic/man8/mkudffs.8.html
> > > is a tool that creates UDF file system with a compatible MBR table in
> > > the top 32KB specially for Windows.
> > >
> > > Search " WHOLE DISK VS PARTITION" in the web page.
> >
> > Thanks -- and, indeed, this confirms my point. The issue (or unexpected
> behavior
> > at least) is not specific to Red Hat, it is part of the (most
> > likely: upstream) mkudffs tool. (Case in point, the manual page you're 
> > quoting
> is
> > from an Ubuntu distro.)
> >
> > I think a minimal reproducer using the mkudffs command (and maybe more
> > commands) would be nice in the TianoCore BZ.
> >
> > Anyway, looking at the manual you've identified, it does seem like a gray
> area.
> > It's a conflict between two specifications, apparently. Or perhaps more
> precisely,
> > a bug in the UDF spec. (It seems really strange that a filesystem insists on
> being
> > installed on a complete block device, and not on a partition described in a
> > standard partition table.)
> >
> > The workaround described in the manual is presented as a workaround "for
> > Windows' sake". I don't think that's fair; in my (uneducated?) opinion
> > *any* file system should be happy to live inside a partition. I wouldn't
> "blame"
> > Windows for making an assumption like this.
> >
> > Furthermore, the technical details of the workaround are really annoying:
> >
> > So  the  whole  disk device  and  also first partition on disk
> > points to same sectors.
> >
> > This means that the first partition recursively includes itself -- in other 
> > words,
> it
> > includes the partition *table* that describes itself. :/
> >
> > The manual then says,
> >
> > mkudffs  option --bootarea=mbr put such MBR table for compatibility
> > with Microsoft Windows systems into disk when formatting.
> >
> > So maybe there *is* a bug in the Red Hat ISO generator scripts; I'm not 
> > sure.
> I
> > don't see a reason why 

Re: [edk2-devel] [PATCH v3 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver

2020-06-28 Thread Guomin Jiang
Hi Ard,

When build MinPlatform, it will show 
Edk2Platforms/Platform/Intel/MinPlatformPkg/Pci/Library/PciSegmentInfoLibSimple/PciSegmentInfoLibSimple.c:34:1:
 error: conflicting types for ‘GetPciSegmentInfo’

Could you confirm it?

Thanks
Guomin
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, June 25, 2020 7:25 PM
> To: Sami Mujawar ; devel@edk2.groups.io
> Cc: l...@nuviainc.com; Ni, Ray ;
> alexandru.eli...@arm.com; andre.przyw...@arm.com;
> matteo.carl...@arm.com; laura.more...@arm.com; n...@arm.com
> Subject: Re: [edk2-devel] [PATCH v3 01/15] PcAtChipsetPkg: Add MMIO
> Support to RTC driver
> 
> On 6/24/20 3:34 PM, Sami Mujawar wrote:
> > Some virtual machine managers like Kvmtool emulate the MC146818 RTC
> > controller in the MMIO space so that architectures that do not support
> > I/O Mapped I/O can use the RTC. This patch adds MMIO support to the
> > RTC controller driver.
> >
> > The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
> >If PcdRtcUseMmio is:
> >  TRUE  - Indicates the RTC port registers are in MMIO space.
> >  FALSE - Indicates the RTC port registers are in I/O space.
> >  Default is I/O space.
> >
> > Additionally two new PCDs PcdRtcIndexRegister64 and
> > PcdRtcTargetRegister64 have been introduced to provide the base
> > address for the RTC registers in the MMIO space.
> >
> > When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver
> > converts the pointers to the RTC MMIO registers so that the RTC
> > registers are accessible post ExitBootServices.
> >
> > Signed-off-by: Sami Mujawar 
> 
> Can't we implement this without function pointers? What Leif suggested was
> using STATIC helper functions for read and write, where each one has a test
> for the feature PCD to decide whether to use the IO or the MMIO version.
> 
> Using function pointers is unneeded complexity, especially given the fact
> that we need to fix up their values when the address space is virtually
> remapped by the OS.
> 
> > ---
> >
> > Notes:
> >  v3:
> >  - Make PcdRtcUseMmio a feature PCD.   
> > [Sami]
> >  - Read the RTC MMIO base address from the DT. 
> > [Andre]
> >  - Introduce PCDs for RTC Index and Target register base   
> > [Sami]
> >address in the MMIO space.
> >  - Move RTC MMIO region mapping code to a separate platform
> [Sami]
> >specific library. This library also reads the base addresses
> >for the RTC from DT and configures the RTC Index and Target
> >register PCDs.
> >Ref: https://edk2.groups.io/g/devel/topic/74200905#60307
> >
> >  v2:
> >  - Code review comments incorporated.  
> > [Sami]
> >
> >  v1:
> >  - Add support to read/write from RTC registers using MMIO access
> [Sami]
> >  - Use wrapper functions for RtcRead/Write accessors   
> > [Leif]
> >Ref: https://edk2.groups.io/g/devel/topic/30915281#30695
> >
> >   PcAtChipsetPkg/PcAtChipsetPkg.dec 
> >  |  16 +++
> >   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> >  | 119
> +---
> >   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h
> >  |  31
> +
> >   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c   
> >  |
> 72 +++-
> >
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntime
> Dxe.inf |   8 ++
> >   5 files changed, 230 insertions(+), 16 deletions(-)
> >
> > diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > index
> >
> 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc
> 290c
> > f3bb905e211b 100644
> > --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> > @@ -6,6 +6,7 @@
> >   #
> >   # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.
> >   # Copyright (c) 2017, AMD Inc. All rights reserved.
> > +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.
> >   #
> >   # SPDX-License-Identifier: BSD-2-Clause-Patent
> >   #
> > @@ -41,6 +42,13 @@ [PcdsFeatureFlag]
> > # @Prompt Configure HPET to use MSI.
> >
> >
> gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x00
> 001000
> >
> > +  ## Indicates the RTC port registers are in MMIO space, or in I/O space.
> > +  #  Default is I/O space.
> > +  #   TRUE  - RTC port registers are in MMIO space.
> > +  #   FALSE - RTC port registers are in I/O space.
> > +  # @Prompt RTC port registers use MMIO.
> > +
> > +
> gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x
> 0021
> > +
> >   [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx,
> PcdsPatchableInModule]
> > ## This PCD specifies the base address of the HPET timer.
> > # @Prompt HPET base address.
> > @@ -68,6 +76,14 @@ [PcdsFixedAtBuild, 

Re: [edk2-devel] [patch V2] MdeModulePkg/DisplayEngine: Add Debug message to show mismatch menu info

2020-06-28 Thread Dong, Eric
Reviewed-by: Eric Dong 

> -Original Message-
> From: Bi, Dandan 
> Sent: Sunday, June 28, 2020 10:17 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Dong, Eric ;
> Laszlo Ersek 
> Subject: [patch V2] MdeModulePkg/DisplayEngine: Add Debug message to
> show mismatch menu info
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2326
> 
> Currently when meet mismatch case for one-of and ordered-list menu, just
> show a popup window to indicate mismatch, no more info for debugging.
> This patch is to add more debug message about mismatch menu info which is
> helpful to debug.
> 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Signed-off-by: Dandan Bi 
> ---
> v2:
> 1) Refine debug message info.
> 2) Use gEfiCallerBaseName.to replace hard code "DisplayEngine"
> 3) Handle question and option value based on value type.
> 
>  .../DisplayEngineDxe/ProcessOptions.c | 125 ++
>  1 file changed, 125 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> index e7306f6d04..c02e36a63a 100644
> --- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> +++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> @@ -911,10 +911,120 @@ PasswordProcess (
>FreePool (StringPtr);
> 
>return Status;
>  }
> 
> +/**
> +  Print some debug message about mismatched menu info.
> +
> +  @param  MenuOption The MenuOption for this Question.
> +
> +**/
> +VOID
> +PrintMismatchMenuInfo (
> +  IN  UI_MENU_OPTION  *MenuOption
> +)
> +{
> +  CHAR16  *FormTitleStr;
> +  CHAR16  *FormSetTitleStr;
> +  CHAR16  *OneOfOptionStr;
> +  CHAR16  *QuestionName;
> +  LIST_ENTRY  *Link;
> +  FORM_DISPLAY_ENGINE_STATEMENT   *Question;
> +  EFI_IFR_ORDERED_LIST*OrderList;
> +  UINT8   Index;
> +  EFI_HII_VALUE   HiiValue;
> +  EFI_HII_VALUE   *QuestionValue;
> +  DISPLAY_QUESTION_OPTION *Option;
> +  UINT8   *ValueArray;
> +  UINT8   ValueType;
> +  EFI_IFR_FORM_SET*FormsetBuffer;
> +  UINTN   FormsetBufferSize;
> +
> +  Question = MenuOption->ThisTag;
> +  HiiGetFormSetFromHiiHandle (gFormData->HiiHandle, ,
> + );
> +
> +  FormSetTitleStr = GetToken (FormsetBuffer->FormSetTitle,
> + gFormData->HiiHandle);  FormTitleStr = GetToken (gFormData->FormTitle,
> + gFormData->HiiHandle);
> +
> +  DEBUG ((DEBUG_ERROR, "\n[%a]: Mismatch Formset: Formset Guid
> = %g,  FormSet title = %s\n", gEfiCallerBaseName, 
> >FormSetGuid, FormSetTitleStr));
> +  DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Form   : FormId = %d,  Form
> title = %s.\n", gEfiCallerBaseName, gFormData->FormId, FormTitleStr));
> +
> +  if (Question->OpCode->OpCode == EFI_IFR_ORDERED_LIST_OP) {
> +QuestionName = GetToken (((EFI_IFR_ORDERED_LIST*)MenuOption-
> >ThisTag->OpCode)->Question.Header.Prompt, gFormData->HiiHandle);
> +Link = GetFirstNode (>OptionListHead);
> +Option = DISPLAY_QUESTION_OPTION_FROM_LINK (Link);
> +ValueType = Option->OptionOpCode->Type;
> +DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Error  : OrderedList value in
> the array doesn't match with option value.\n", gEfiCallerBaseName));
> +DEBUG ((DEBUG_ERROR, "[%a]: Mismatch OrderedList: Name = %s.\n",
> gEfiCallerBaseName, QuestionName));
> +DEBUG ((DEBUG_ERROR, "[%a]: Mismatch OrderedList: OrderedList array
> + value :\n", gEfiCallerBaseName));
> +
> +OrderList = (EFI_IFR_ORDERED_LIST *) Question->OpCode;
> +for (Index = 0; Index < OrderList->MaxContainers; Index++) {
> +  ValueArray = Question->CurrentValue.Buffer;
> +  HiiValue.Value.u64 = GetArrayData (ValueArray, ValueType, Index);
> +  DEBUG ((DEBUG_ERROR, "   Value[%d] 
> =%ld.\n", Index,
> HiiValue.Value.u64));
> +}
> +  } else if (Question->OpCode->OpCode == EFI_IFR_ONE_OF_OP) {
> +QuestionName = GetToken (((EFI_IFR_ONE_OF*)MenuOption->ThisTag-
> >OpCode)->Question.Header.Prompt, gFormData->HiiHandle);
> +QuestionValue = >CurrentValue;
> +DEBUG ((DEBUG_ERROR, "[%a]: Mismatch Error  : OneOf value doesn't
> match with option value.\n", gEfiCallerBaseName));
> +DEBUG ((DEBUG_ERROR, "[%a]: Mismatch OneOf  : Name = %s.\n",
> gEfiCallerBaseName, QuestionName));
> +switch (QuestionValue->Type) {
> +  case EFI_IFR_TYPE_NUM_SIZE_64:
> +DEBUG ((DEBUG_ERROR, "[%a]: Mismatch OneOf  : OneOf value
> = %ld.\n",gEfiCallerBaseName, QuestionValue->Value.u64));
> +break;
> +
> +  case EFI_IFR_TYPE_NUM_SIZE_32:
> +DEBUG ((DEBUG_ERROR, "[%a]: Mismatch OneOf  : OneOf value
> = %d.\n",gEfiCallerBaseName, QuestionValue->Value.u32));
> +break;
> +
> +  case 

Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf handler

2020-06-28 Thread Gao, Zhichao
Hi Laszlo,

Sorry, I didn't put the detail info about the issue. Let me descript here.

The issue is not only for Red Hat. I found it with ubuntu 18.02 amd64 and 
Fedora-20-x86_64 ISO image as well. I didn't view all the linux iso images.

Here is the issue:
Using USB CD ROM with the linux ISO DVD. Run the platform and enter UEFI shell 
-> plug the USB CD ROM with the ISO DVD -> run "map -r". The CD's file system 
didn't show.

It works fine with the normal boot (such as F2, F7 and auto boot) because it 
would run the *connect all* operation. That would run the partition driver 
serval times. First time the partition driver would install MBR partition info 
protocol with the device handle and skip the UDF check. The second time, it 
would fail the MBR check with *already started* and run UDF check next. That 
means for such ISO the UEFI would install two partition protocol, both MBR and 
UDF.

But for shell environment, the USB hot plug handle would connect the USB device 
only once and missing the UDF check.

I don't know why linux need the MBR info. Maybe for legacy compatible thinking 
(my opinion, may be wrong).

If the GPT, MBR and UDF are conflict, then the original logic is fine. But in 
fact, GPT and MBR are conflict and GPT/MBR and UDF are not conflict.

Thanks,
Zhichao

> -Original Message-
> From: Laszlo Ersek 
> Sent: Thursday, June 25, 2020 5:02 PM
> To: Ni, Ray ; devel@edk2.groups.io; Gao, Zhichao
> 
> Cc: Wu, Hao A ; Wang, Jian J ;
> Gao, Liming 
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf
> handler
> 
> On 06/25/20 02:21, Ni, Ray wrote:
> > Laszlo,
> > You are surprised because the issue is related to Redhat, or because
> > the word "Redhat" should not be mentioned in the Bugzilla?
> 
> It's totally fine to mention Red Hat (with correct spelling) in the issue 
> description.
> The more details, the better.
> 
> I'm surprised that the issue *seems* specific to Red Hat. Earlier I had 
> looked on-
> and-off on the scripts that generate (for example) the Fedora ISO images (so 
> that
> they be both BIOS-bootable and UEFI-bootable), and I don't recall any 
> particular
> tricks that would exploit gray areas in specs or so.
> 
> It's possible that the *tools* the scripts use for formatting the ISOs have 
> some
> bugs or quirks, of course. (But then I'm again a bit surprised for those 
> issues or
> quirks to be RH-specific -- I just don't see a reason for us to diverge from 
> the
> respective upstream code bases of those tools.)
> 
> > http://manpages.ubuntu.com/manpages/bionic/man8/mkudffs.8.html
> > is a tool that creates UDF file system with a compatible MBR table in
> > the top 32KB specially for Windows.
> >
> > Search " WHOLE DISK VS PARTITION" in the web page.
> 
> Thanks -- and, indeed, this confirms my point. The issue (or unexpected 
> behavior
> at least) is not specific to Red Hat, it is part of the (most
> likely: upstream) mkudffs tool. (Case in point, the manual page you're 
> quoting is
> from an Ubuntu distro.)
> 
> I think a minimal reproducer using the mkudffs command (and maybe more
> commands) would be nice in the TianoCore BZ.
> 
> Anyway, looking at the manual you've identified, it does seem like a gray 
> area.
> It's a conflict between two specifications, apparently. Or perhaps more 
> precisely,
> a bug in the UDF spec. (It seems really strange that a filesystem insists on 
> being
> installed on a complete block device, and not on a partition described in a
> standard partition table.)
> 
> The workaround described in the manual is presented as a workaround "for
> Windows' sake". I don't think that's fair; in my (uneducated?) opinion
> *any* file system should be happy to live inside a partition. I wouldn't 
> "blame"
> Windows for making an assumption like this.
> 
> Furthermore, the technical details of the workaround are really annoying:
> 
> So  the  whole  disk device  and  also first partition on disk
> points to same sectors.
> 
> This means that the first partition recursively includes itself -- in other 
> words, it
> includes the partition *table* that describes itself. :/
> 
> The manual then says,
> 
> mkudffs  option --bootarea=mbr put such MBR table for compatibility
> with Microsoft Windows systems into disk when formatting.
> 
> So maybe there *is* a bug in the Red Hat ISO generator scripts; I'm not sure. 
> I
> don't see a reason why "--bootarea=mbr" should ever be used with
> *removable* media (such as an ISO image). The mkudffs manual states that this
> trickery is useful only for non-removable media even on Windows:
> 
> Microsoft Windows systems are unable to detect non-removable hard
> disks without MBR or GPT partition table.  Removable disks do not
> have this restriction.
> 
> (Personally I've been using an UDF-formatted USB pendrive for a long time now,
> because it supports 4G+ files, and is compatible with both Linux and Windows.
> There's no partition table on it; I 

Re: [edk2-devel] [PATCH v4 13/17] PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml: Add configuration for Ecc check

2020-06-28 Thread Guomin Jiang
Good work.

Reviewed-by: Guomin Jiang 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhang,
> Shenglei
> Sent: Monday, June 15, 2020 4:49 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray 
> Subject: [edk2-devel] [PATCH v4 13/17]
> PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml: Add configuration for Ecc check
> 
> Add configuration ExceptionList and IgnoreFiles for package config files. So
> users can rely on this to ignore some Ecc issues.
> 
> Cc: Ray Ni 
> Signed-off-by: Shenglei Zhang 
> ---
>  PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml
> b/PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml
> index be470807bd9e..5809ff8e3b1c 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.ci.yaml
> @@ -5,6 +5,16 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  ##  {
> +"EccCheck": {
> +## Exception sample looks like below:
> +## "ExceptionList": [
> +## "", ""
> +## ]
> +"ExceptionList": [
> +],
> +"IgnoreFiles": [
> +]
> +},
>  "CompilerPlugin": {
>  "DscPath": "PcAtChipsetPkg.dsc"
>  },
> --
> 2.18.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61776): https://edk2.groups.io/g/devel/message/61776
Mute This Topic: https://groups.io/mt/74890559/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v4 08/17] FatPkg/FatPkg.ci.yaml: Add configuration for Ecc check

2020-06-28 Thread Guomin Jiang
Good work.

Reviewed-by: Guomin Jiang 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhang,
> Shenglei
> Sent: Monday, June 15, 2020 4:49 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray 
> Subject: [edk2-devel] [PATCH v4 08/17] FatPkg/FatPkg.ci.yaml: Add
> configuration for Ecc check
> 
> Add configuration ExceptionList and IgnoreFiles for package config files. So
> users can rely on this to ignore some Ecc issues.
> 
> Cc: Ray Ni 
> Signed-off-by: Shenglei Zhang 
> ---
>  FatPkg/FatPkg.ci.yaml | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/FatPkg/FatPkg.ci.yaml b/FatPkg/FatPkg.ci.yaml index
> 8b0fb1d4fcd5..82069d3a7534 100644
> --- a/FatPkg/FatPkg.ci.yaml
> +++ b/FatPkg/FatPkg.ci.yaml
> @@ -5,6 +5,16 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  ##  {
> +"EccCheck": {
> +## Exception sample looks like below:
> +## "ExceptionList": [
> +## "", ""
> +## ]
> +"ExceptionList": [
> +],
> +"IgnoreFiles": [
> +]
> +},
>  "CompilerPlugin": {
>  "DscPath": "FatPkg.dsc"
>  },
> --
> 2.18.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61775): https://edk2.groups.io/g/devel/message/61775
Mute This Topic: https://groups.io/mt/74890554/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v4 09/17] FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration for Ecc check

2020-06-28 Thread Guomin Jiang
Good work.

Reviewed-by: Guomin Jiang 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhang,
> Shenglei
> Sent: Monday, June 15, 2020 4:49 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Kinney, Michael D
> 
> Subject: [edk2-devel] [PATCH v4 09/17]
> FmpDevicePkg/FmpDevicePkg.ci.yaml: Add configuration for Ecc check
> 
> Add configuration ExceptionList and IgnoreFiles for package config files. So
> users can rely on this to ignore some Ecc issues.
> 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Signed-off-by: Shenglei Zhang 
> ---
>  FmpDevicePkg/FmpDevicePkg.ci.yaml | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/FmpDevicePkg/FmpDevicePkg.ci.yaml
> b/FmpDevicePkg/FmpDevicePkg.ci.yaml
> index 74a0aefe8e49..99ffa4aae8f2 100644
> --- a/FmpDevicePkg/FmpDevicePkg.ci.yaml
> +++ b/FmpDevicePkg/FmpDevicePkg.ci.yaml
> @@ -5,6 +5,16 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  ##  {
> +"EccCheck": {
> +## Exception sample looks like below:
> +## "ExceptionList": [
> +## "", ""
> +## ]
> +"ExceptionList": [
> +],
> +"IgnoreFiles": [
> +]
> +},
>  "CompilerPlugin": {
>  "DscPath": "FmpDevicePkg.dsc"
>  },
> --
> 2.18.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61774): https://edk2.groups.io/g/devel/message/61774
Mute This Topic: https://groups.io/mt/74890555/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v4 06/17] CryptoPkg/CryptoPkg.ci.yaml: Add configuration for Ecc check

2020-06-28 Thread Guomin Jiang
Good work.

Reviewed-by: Guomin Jiang 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhang,
> Shenglei
> Sent: Monday, June 15, 2020 4:49 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J ; Lu, XiaoyuX
> 
> Subject: [edk2-devel] [PATCH v4 06/17] CryptoPkg/CryptoPkg.ci.yaml: Add
> configuration for Ecc check
> 
> Add configuration ExceptionList and IgnoreFiles for package config files. So
> users can rely on this to ignore some Ecc issues.
> 
> Cc: Jian J Wang 
> Cc: Xiaoyu Lu 
> Signed-off-by: Shenglei Zhang 
> ---
>  CryptoPkg/CryptoPkg.ci.yaml | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/CryptoPkg/CryptoPkg.ci.yaml b/CryptoPkg/CryptoPkg.ci.yaml
> index f54ebfb22e70..2c1a8a7f5072 100644
> --- a/CryptoPkg/CryptoPkg.ci.yaml
> +++ b/CryptoPkg/CryptoPkg.ci.yaml
> @@ -5,6 +5,16 @@
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  ##  {
> +"EccCheck": {
> +## Exception sample looks like below:
> +## "ExceptionList": [
> +## "", ""
> +## ]
> +"ExceptionList": [
> +],
> +"IgnoreFiles": [
> +]
> +},
>  "CompilerPlugin": {
>  "DscPath": "CryptoPkg.dsc"
>  },
> --
> 2.18.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61773): https://edk2.groups.io/g/devel/message/61773
Mute This Topic: https://groups.io/mt/74890552/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 1/1] IntelFsp2Pkg/Tools/GenCfgOpt.py: Fix a bug about parse macro

2020-06-28 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 

> -Original Message-
> From: Tan, Ming 
> Sent: Sunday, June 28, 2020 2:54 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star 
> Subject: [PATCH 1/1] IntelFsp2Pkg/Tools/GenCfgOpt.py: Fix a bug about parse
> macro
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2827
> 
> Fix a bug about parse the macro value which use another macro.
> 
> Use the following example to verify:
> [Define]
>   DEFINE M1 = V1
>   DEFINE M2 = $(M1)/V2
> 
>   !include $(M2)/pcd.dsc
> 
> The old code will failed parse M2 and cause following error:
> Traceback (most recent call last):
>   File "Edk2\IntelFsp2Pkg\Tools\GenCfgOpt.py", line 1550, in 
> sys.exit(Main())
>   File "Edk2\IntelFsp2Pkg\Tools\GenCfgOpt.py", line 1513, in Main
> if GenCfgOpt.ParseDscFile(DscFile, FvDir) != 0:
>   File "Edk2\IntelFsp2Pkg\Tools\GenCfgOpt.py", line 533, in ParseDscFile
> NewDscLines = IncludeDsc.readlines()
> ValueError: I/O operation on closed file.
> 
> The tool should support the value use another macro, and expand it.
> 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Star Zeng 
> Signed-off-by: Ming Tan 
> ---
>  IntelFsp2Pkg/Tools/GenCfgOpt.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> b/IntelFsp2Pkg/Tools/GenCfgOpt.py index e6c15108f5c5..e9de128e5055
> 100644
> --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> @@ -546,11 +546,11 @@ EndList
>  #DEFINE FSP_T_UPD_TOOL_GUID =
> 34686CA3-34F9-4901-B82A-BA630F0714C6
>  #DEFINE FSP_M_UPD_TOOL_GUID =
> 39A250DB-E465-4DD1-A2AC-E2BD3C0E2385
>  #DEFINE FSP_S_UPD_TOOL_GUID =
> CAE3605B-5B34-4C85-B3D7-27D54273C40F
> -Match =
> re.match("^\s*(?:DEFINE\s+)*(\w+)\s*=\s*([-.\w]+)", DscLine)
> +Match =
> + re.match("^\s*(?:DEFINE\s+)*(\w+)\s*=\s*([/$()-.\w]+)", DscLine)
>  if Match:
> -self._MacroDict[Match.group(1)] = Match.group(2)
> +self._MacroDict[Match.group(1)] =
> + self.ExpandMacros(Match.group(2))
>  if self.Debug:
> -print ("INFO : DEFINE %s = [ %s ]" %
> (Match.group(1), Match.group(2)))
> +print ("INFO : DEFINE %s = [ %s ]" %
> + (Match.group(1), self.ExpandMacros(Match.group(2
>  elif IsPcdSect:
>  #gSiPkgTokenSpaceGuid.PcdTxtEnable|FALSE
>  #gSiPkgTokenSpaceGuid.PcdOverclockEnable|TRUE
> --
> 2.24.0.windows.2


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61772): https://edk2.groups.io/g/devel/message/61772
Mute This Topic: https://groups.io/mt/75166595/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH] IntelFsp2Pkg/FspSecCore: Use UefiCpuLib.

2020-06-28 Thread Chiu, Chasel


Reviewed-by: Chasel Chiu 

> -Original Message-
> From: Dong, Eric 
> Sent: Saturday, June 27, 2020 9:52 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star 
> Subject: [PATCH] IntelFsp2Pkg/FspSecCore: Use UefiCpuLib.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2825
> 
> UefiCpuLib has API InitializeFloatingPointUnits.
> Remove internal copy of InitializeFloatingPointUnits in FspSecCoreM, use
> UefiCpuLib API.
> 
> This change also avoid later potential conflict when use UefiCpuLib for
> FspSecCoreM module.
> 
> Signed-off-by: Eric Dong 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Star Zeng 
> ---
>  IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf   |  3 +-
>  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf   |  1 -
>  .../FspSecCore/Ia32/InitializeFpu.nasm| 72 ---
>  IntelFsp2Pkg/FspSecCore/SecMain.h | 15 +---
>  IntelFsp2Pkg/IntelFsp2Pkg.dsc |  1 +
>  5 files changed, 4 insertions(+), 88 deletions(-)  delete mode 100644
> IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> index 25f2a109ab..61b7ddca4c 100644
> --- a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> @@ -29,7 +29,6 @@
> 
>  [Sources.IA32]
>Ia32/Stack.nasm
> -  Ia32/InitializeFpu.nasm
>Ia32/FspApiEntryM.nasm
>Ia32/FspApiEntryCommon.nasm
>Ia32/FspHelper.nasm
> @@ -41,6 +40,7 @@
>  [Packages]
>MdePkg/MdePkg.dec
>IntelFsp2Pkg/IntelFsp2Pkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> 
>  [LibraryClasses]
>BaseMemoryLib
> @@ -51,6 +51,7 @@
>FspSwitchStackLib
>FspCommonLib
>FspSecPlatformLib
> +  UefiCpuLib
> 
>  [Pcd]
>gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamBase  ##
> CONSUMES
> diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> b/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> index 971b311e42..664bde5678 100644
> --- a/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> @@ -25,7 +25,6 @@
> 
>  [Sources.IA32]
>Ia32/Stack.nasm
> -  Ia32/InitializeFpu.nasm
>Ia32/FspApiEntryT.nasm
>Ia32/FspHelper.nasm
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> deleted file mode 100644
> index ebc91c41e4..00
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/InitializeFpu.nasm
> +++ /dev/null
> @@ -1,72 +0,0 @@
> -;--
> -;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved. -;
> SPDX-License-Identifier: BSD-2-Clause-Patent -; -; Abstract:
> -;
> -;--
> -
> -
> -SECTION .data
> -;
> -; Float control word initial value:
> -; all exceptions masked, double-precision, round-to-nearest -;
> -ASM_PFX(mFpuControlWord):
> -dw0x027F
> -;
> -; Multimedia-extensions control word:
> -; all exceptions masked, round-to-nearest, flush to zero for masked
> underflow -;
> -ASM_PFX(mMmxControlWord):
> - dd 0x01F80
> -
> -SECTION .text
> -
> -;
> -; Initializes floating point units for requirement of UEFI specification.
> -;
> -; This function initializes floating-point control word to 0x027F (all
> exceptions -; masked,double-precision, round-to-nearest) and
> multimedia-extensions control word -; (if supported) to 0x1F80 (all
> exceptions masked, round-to-nearest, flush to zero -; for masked underflow).
> -;
> -
> -global ASM_PFX(InitializeFloatingPointUnits)
> -ASM_PFX(InitializeFloatingPointUnits):
> -
> -
> -pushebx
> -
> -;
> -; Initialize floating point units
> -;
> -finit
> -fldcw[ASM_PFX(mFpuControlWord)]
> -
> -;
> -; Use CpuId instruction (CPUID.01H:EDX.SSE[bit 25] = 1) to test
> -; whether the processor supports SSE instruction.
> -;
> -mov eax, 1
> -cpuid
> -bt  edx, 25
> -jnc Done
> -
> -;
> -; Set OSFXSR bit 9 in CR4
> -;
> -mov eax, cr4
> -or  eax, BIT9
> -mov cr4, eax
> -
> -;
> -; The processor should support SSE instruction and we can use
> -; ldmxcsr instruction
> -;
> -ldmxcsr [ASM_PFX(mMmxControlWord)]
> -Done:
> -pop ebx
> -
> -ret
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.h
> b/IntelFsp2Pkg/FspSecCore/SecMain.h
> index af7f387960..f6333b0ffb 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.h
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.h
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> 
>  typedef VOID (*PEI_CORE_ENTRY) ( \
> @@ -80,20 +81,6 @@ SecTemporaryRamSupport (
>IN UINTNCopySize
>);
> 
> -/**
> -  Initializes floating point units for requirement of UEFI specification.
> -
> -  This function initializes floating-point control word to 0x027F (all
> 

Re: [edk2-devel] [PATCH] BaseTools/build.py: Exit with 1 when AutoGen error occurred

2020-06-28 Thread Yuwei Chen
Reivewed-by: Yuwei Chen

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Irene
> Park
> Sent: Wednesday, June 3, 2020 5:59 AM
> To: devel@edk2.groups.io
> Cc: Irene Park 
> Subject: [edk2-devel] [PATCH] BaseTools/build.py: Exit with 1 when AutoGen
> error occurred
> 
> From: Irene Park 
> 
> AutoGen manager/workers halt the progress when an error occurs but
> doesn't propagate the error code to main and allows main exit with 0 and
> gets the build system unable to catch the occurrence of an error.
> This change informs main with an error when a progress is halted and helps
> main exit with 1.
> 
> Signed-off-by: Irene Park 
> ---
>  BaseTools/Source/Python/build/build.py | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/build/build.py
> b/BaseTools/Source/Python/build/build.py
> index ed3a3b9..1ab1e60 100755
> --- a/BaseTools/Source/Python/build/build.py
> +++ b/BaseTools/Source/Python/build/build.py
> @@ -880,7 +880,10 @@ class Build():
> 
>  self.AutoGenMgr.join()
>  rt = self.AutoGenMgr.Status
> -return rt, 0
> +err = 0
> +if not rt:
> +err = UNKNOWN_ERROR
> +return rt, err
>  except FatalError as e:
>  return False, e.args[0]
>  except:
> @@ -2724,4 +2727,3 @@ if __name__ == '__main__':
>  ## 0-127 is a safe return range, and 1 is a standard default error
>  if r < 0 or r > 127: r = 1
>  sys.exit(r)
> -
> --
> 2.7.4
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61770): https://edk2.groups.io/g/devel/message/61770
Mute This Topic: https://groups.io/mt/74638295/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v3 0/4] Compile AML bytecode array into OBJ file

2020-06-28 Thread PierreGondois
Hello Bob,
The PR729 didn't pass the PatchCheck for the patch adding the LF line endings 
to the script in BaseTools/BinWrappers/PosixLike, this is for the patch named 
"BaseTools: Rename AmlToHex script to AmlToC" (cf the following link: 
https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=8860=logs=12f1170f-54f2-53f3-20dd-22fc7dff55f9=9c939e41-62c2-5605-5e05-fc3554afc9f5=76).
However, I thought it was a normal behaviour. Indeed, PatchCheck.py is 
triggering an error for any file with LF line endings, except when:
 - the file has a ".sh" extension
 - the filename is in .gitmodules (it is coming from another git repo)
 - the file mode of the file is 16  (it is coming from another git repo)
The new AmlToC posix script doesn't match any of these conditions. However some 
other files are in the same conditions. I searched the files containing the 
"#!usr/bin/env" string and which don't have a ".sh" extension. I tried to run 
the PatchCheck.py script on their respective commit with the version of 
PatchCheck.py that existed at that time and the same CRLF error appeared. The 
list of these files is at the end of the mail. Renaming them with a ".sh" 
extension would break the build on linux: the extension of the file is 
important to locate a file. This is not the case on windows (and this explains 
why all the scripts in BaseTools\BinWrappers\WindowsLike have a .bat extension).
All the bash scripts not having a ".sh" extension are located in 
BaseTools/Bin/CYGWIN_NT-5.1-i686/ or BaseTools/BinWrappers/PosixLike/ . A 
solution would be to add an exception to the CRLF check for these two folders.

A small question: when applying the patch set, did 
BaseTools\BinWrappers\PosixLike\AmlToC have CRLF line endings and you had to 
manually modify them to linux line endings, or where the line endings already 
correct (linux LF line endings)?

Regards,
Pierre

The list of bash scripts without a ".sh" extension:
BaseTools/Bin/CYGWIN_NT-5.1-i686/BootSectImage
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/build
commit 7aef7b7cbf16f79fb17c5ace98b1bc7f15bb90fa
Date:   2018-12-28T16:25:04+08:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/BuildEnv
commit 66a2dc96d3dff90b4243c4ed3e7eaa33abdcdf3c
Date:   2012-02-15T08:06:01+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/Ecc
commit 7aef7b7cbf16f79fb17c5ace98b1bc7f15bb90fa
Date:   2018-12-28T16:25:04+08:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/EfiLdrImage
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/EfiRom
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenCrc32
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenDepex
commit 7aef7b7cbf16f79fb17c5ace98b1bc7f15bb90fa
Date:   2018-12-28T16:25:04+08:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenFds
commit 7aef7b7cbf16f79fb17c5ace98b1bc7f15bb90fa
Date:   2018-12-28T16:25:04+08:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenFfs
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenFv
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenFw
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenPage
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenSec
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GenVtf
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/GnuGenBootSector
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/LzmaCompress
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/LzmaF86Compress
commit 2e351cbe8e190271b3716284fc1076551d005472
Date:   2019-04-03T16:03:11-07:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/RunBinToolFromBuildDir
commit 66a2dc96d3dff90b4243c4ed3e7eaa33abdcdf3c
Date:   2012-02-15T08:06:01+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/RunToolFromSource
commit 66a2dc96d3dff90b4243c4ed3e7eaa33abdcdf3c
Date:   2012-02-15T08:06:01+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/Split
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/TargetTool
commit 7aef7b7cbf16f79fb17c5ace98b1bc7f15bb90fa
Date:   2018-12-28T16:25:04+08:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/TianoCompress
commit 5e407648358d639ed19d228303527133e4c95c25
Date:   2016-01-20T05:12:02+00:00

BaseTools/Bin/CYGWIN_NT-5.1-i686/Trim
commit 7aef7b7cbf16f79fb17c5ace98b1bc7f15bb90fa
Date:   2018-12-28T16:25:04+08:00


Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

2020-06-28 Thread Dong, Eric
Hi Guomin, Laszlo & Garrett,

Because both UefiCpuLib and FspSecCoreT / FspSecCoreM both define 
InitializeFloatingPointUnits API. So this build issue been reported.

I have submit a patch to remove InitializeFloatingPointUnits in FspSecCoreT / 
FspSecCoreM, https://edk2.groups.io/g/devel/message/61757.

Please wait that patch been merged before committing this change.

Thanks,
Eric
> -Original Message-
> From: Jiang, Guomin 
> Sent: Sunday, June 28, 2020 5:11 PM
> To: Laszlo Ersek ; devel@edk2.groups.io;
> garrett.kirkend...@amd.com
> Cc: Ni, Ray ; Dong, Eric 
> Subject: RE: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg:
> PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
> 
> Hi Laszlo,
> 
> The step to reproduce as below:
> 1. add below change
> diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> index 26cd3da43c3f..d43cb5be6472 100644
> --- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
> @@ -62,7 +62,10 @@ [Components]
>IntelFsp2Pkg/Library/BaseDebugDeviceLibNull/BaseDebugDeviceLibNull.inf
> 
> IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/SecFspSecPlatformLibNull.in
> f
> 
> -  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
> +  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf {
> +
> +NULL|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> +  }
>IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
>IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
>IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf
> 
> 2. build -p IntelFsp2Pkg\IntelFsp2Pkg.dsc -b DEBUG -a IA32 -t VS2019 and I
> can see BaseUefiCpuLib.lib(InitializeFpu.obj) : error LNK2005:
> _InitializeFloatingPointUnits already defined in
> FspSecCoreT.lib(InitializeFpu.obj)
> 
> I can't reproduce it with the original edk2 or edk2-platforms, but our project
> have the depend on ApicLib, so if the ApicLib depend on UefiCpuLib, it will
> break our project build.
> 
> Below
> Thanks.
> 
> > -Original Message-
> > From: Laszlo Ersek 
> > Sent: Tuesday, June 23, 2020 7:16 PM
> > To: devel@edk2.groups.io; Jiang, Guomin ;
> > garrett.kirkend...@amd.com
> > Cc: Ni, Ray ; Dong, Eric 
> > Subject: Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg:
> > PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
> >
> > On 06/23/20 02:52, Guomin Jiang wrote:
> > > Hi Garrett,
> > >
> > > Thanks for report quickly and it work and the 'Mac format issue
> disappear'.
> > >
> > > But another build error occur 'error LNK2005:
> > > _InitializeFloatingPointUnits
> > already defined in FspSecCoreT.lib(InitializeFpu.obj)'
> > > It seem that result by ApicLib who depend UefiCpuLib now.
> >
> > Yes.
> >
> > > I want to know why you add this dependency,
> >
> > It was discussed on the list in advance.
> >
> > [edk2-devel] UefiCpuPkg: Discuss: Move
> StandardSignatureIsAuthenticAMD
> > function to BaseUefiCpuLib
> >
> >   https://edk2.groups.io/g/devel/message/61079
> >
> > In addition, we have investigated all the platforms in the open source
> > edk2 and edk2-platforms trees that could require an update due to the
> > new
> > dependency:
> >
> >   https://edk2.groups.io/g/devel/message/61525
> >
> > And this series is in fact addressing them all.
> >
> > > have any other way to do it, for example, add the dependency to the
> > > dsc
> > file.
> >
> > Please describe how you can reproduce the link error using edk2 and
> > edk2- platforms content *only* (= using open source components only).
> >
> > Thanks
> > Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61768): https://edk2.groups.io/g/devel/message/61768
Mute This Topic: https://groups.io/mt/75037835/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg: PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass

2020-06-28 Thread Guomin Jiang
Hi Laszlo,

The step to reproduce as below:
1. add below change
diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dsc b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
index 26cd3da43c3f..d43cb5be6472 100644
--- a/IntelFsp2Pkg/IntelFsp2Pkg.dsc
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.dsc
@@ -62,7 +62,10 @@ [Components]
   IntelFsp2Pkg/Library/BaseDebugDeviceLibNull/BaseDebugDeviceLibNull.inf
   IntelFsp2Pkg/Library/SecFspSecPlatformLibNull/SecFspSecPlatformLibNull.inf

-  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf
+  IntelFsp2Pkg/FspSecCore/FspSecCoreT.inf {
+
+NULL|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
+  }
   IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
   IntelFsp2Pkg/FspSecCore/FspSecCoreS.inf
   IntelFsp2Pkg/FspSecCore/Fsp22SecCoreS.inf

2. build -p IntelFsp2Pkg\IntelFsp2Pkg.dsc -b DEBUG -a IA32 -t VS2019 and I can 
see BaseUefiCpuLib.lib(InitializeFpu.obj) : error LNK2005: 
_InitializeFloatingPointUnits already defined in 
FspSecCoreT.lib(InitializeFpu.obj)

I can't reproduce it with the original edk2 or edk2-platforms, but our project 
have the depend on ApicLib, so if the ApicLib depend on UefiCpuLib, it will 
break our project build.

Below
Thanks.

> -Original Message-
> From: Laszlo Ersek 
> Sent: Tuesday, June 23, 2020 7:16 PM
> To: devel@edk2.groups.io; Jiang, Guomin ;
> garrett.kirkend...@amd.com
> Cc: Ni, Ray ; Dong, Eric 
> Subject: Re: [edk2-devel] [PATCH v6 1/4] PcAtChipsetPkg:
> PcAtChipsetPkg.dsc add UefiCpuLib LibraryClass
> 
> On 06/23/20 02:52, Guomin Jiang wrote:
> > Hi Garrett,
> >
> > Thanks for report quickly and it work and the 'Mac format issue disappear'.
> >
> > But another build error occur 'error LNK2005: _InitializeFloatingPointUnits
> already defined in FspSecCoreT.lib(InitializeFpu.obj)'
> > It seem that result by ApicLib who depend UefiCpuLib now.
> 
> Yes.
> 
> > I want to know why you add this dependency,
> 
> It was discussed on the list in advance.
> 
> [edk2-devel] UefiCpuPkg: Discuss: Move StandardSignatureIsAuthenticAMD
> function to BaseUefiCpuLib
> 
>   https://edk2.groups.io/g/devel/message/61079
> 
> In addition, we have investigated all the platforms in the open source
> edk2 and edk2-platforms trees that could require an update due to the new
> dependency:
> 
>   https://edk2.groups.io/g/devel/message/61525
> 
> And this series is in fact addressing them all.
> 
> > have any other way to do it, for example, add the dependency to the dsc
> file.
> 
> Please describe how you can reproduce the link error using edk2 and edk2-
> platforms content *only* (= using open source components only).
> 
> Thanks
> Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61767): https://edk2.groups.io/g/devel/message/61767
Mute This Topic: https://groups.io/mt/75037835/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v3 0/4] Compile AML bytecode array into OBJ file

2020-06-28 Thread Bob Feng
Hi Pierre,

I met a problem when I push your patch set.

If I change the CRLF to a unix format EOL, the patch can't pass PatchCheck.py.
If I don't change the CRLF, The build on Linux will fail.
So I can't make the patch set pass the CI.

Could you share me how did you do to make your PR 729 pass?

Thanks,
Bob

-Original Message-
From: devel@edk2.groups.io  On Behalf Of PierreGondois
Sent: Thursday, June 25, 2020 5:31 PM
To: devel@edk2.groups.io; Feng, Bob C 
Cc: Sami Mujawar ; Tomas Pilar ; 
Gao, Liming ; nd 
Subject: Re: [edk2-devel] [PATCH v3 0/4] Compile AML bytecode array into OBJ 
file

Hello Bob,
I believe the line endings of the BaseTools/BinWrappers/PosixLike/AmlToC script 
have been modified to CRLF when I sent the patch. I created a pull request from 
the linked github branch noted in the patches. It is available at 
https://github.com/PierreARM/edk2/commits/803_Compile_AML_bytecode_array_into_OBJ_file_v3
 . The pull request is available here (to show the result of the CI tests) 
https://github.com/tianocore/edk2/pull/729 .
Do you want a v4 or is it possible to pull the patches from the github 
repository?
Sorry for the inconvenience.

Regards,
Pierre

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Bob Feng via 
groups.io
Sent: 24 June 2020 16:16
To: devel@edk2.groups.io; Pierre Gondois 
Cc: Sami Mujawar ; Tomas Pilar ; 
Gao, Liming ; nd 
Subject: Re: [edk2-devel] [PATCH v3 0/4] Compile AML bytecode array into OBJ 
file

Hi Pierre,

There are some build failed in OpenCI. Would you please check it?
https://github.com/tianocore/edk2/pull/727

Thanks,
Bob

-Original Message-
From: devel@edk2.groups.io  On Behalf Of PierreGondois
Sent: Wednesday, June 24, 2020 5:09 PM
To: devel@edk2.groups.io
Cc: Pierre Gondois ; sami.muja...@arm.com; 
tomas.pi...@arm.com; Feng, Bob C ; Gao, Liming 
; n...@arm.com
Subject: [edk2-devel] [PATCH v3 0/4] Compile AML bytecode array into OBJ file

Following the BZ at https://bugzilla.tianocore.org/show_bug.cgi?id=2425
This patch serie is a another way to solve the dependency of C files over ASL 
files. With this new method, the dependency is resolved at the linking stage.

The last method to solve this dependency was to add the possibility to modify 
INF files to depict such a dependency. This method was not accepted. The 
discussion is available at https://edk2.groups.io/g/devel/topic/72655342#56658

The last patch modifying the INF specification and INF parsing are available at:
https://edk2.groups.io/g/devel/topic/72655342#56658
https://edk2.groups.io/g/devel/topic/72656060#56662

Pierre Gondois (4):
  BaseTools: Generate multiple rules when multiple output files
  BaseTools: Rename AmlToHex script to AmlToC
  BaseTools: Compile AML bytecode arrays into .obj file
  BaseTools: Fix string concatenation

 BaseTools/BinWrappers/PosixLike/{AmlToHex => AmlToC}   | 28 +++
 BaseTools/BinWrappers/WindowsLike/{AmlToHex.bat => AmlToC.bat} |  0
 BaseTools/Conf/build_rule.template | 15 +++-
 BaseTools/Source/Python/{AmlToHex/AmlToHex.py => AmlToC/AmlToC.py} | 82 

 BaseTools/Source/Python/AutoGen/BuildEngine.py |  2 +-
 BaseTools/Source/Python/AutoGen/GenMake.py |  6 ++
 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py   | 38 
+
 7 files changed, 89 insertions(+), 82 deletions(-)  rename 
BaseTools/BinWrappers/PosixLike/{AmlToHex => AmlToC} (97%)  rename 
BaseTools/BinWrappers/WindowsLike/{AmlToHex.bat => AmlToC.bat} (100%)  rename 
BaseTools/Source/Python/{AmlToHex/AmlToHex.py => AmlToC/AmlToC.py} (52%)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'











-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61766): https://edk2.groups.io/g/devel/message/61766
Mute This Topic: https://groups.io/mt/75078123/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH V2] MdePkg: Add Serial Terminal Device Type Guid

2020-06-28 Thread Zhiguang Liu
Hi Oleksiy,

If you build OVMF with your code change, it will report one error:
error 3000: Invalid GUID value format
gEfiSerialTerminalDeviceTypeGuid = { 0x6AD9A60F, 0x5815, 0x4C7C, { 
0x08A, 0x10, 0x50, 0x53, 0xD2, 0xBF, 0x7A, 0x1B }} ( = 
)

This is because you use "0x08A" instead of "0x8A", and the tool thought you are 
trying to write a 2 bytes number 008A, not 1-byte number 8A.

Please fix it by just using "0x8A"

Thanks
Zhiguang


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Oleksiy
> Yakovlev
> Sent: Thursday, June 25, 2020 12:20 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming ; Kinney, Michael D
> ; fel...@ami.com; oleks...@ami.com
> Subject: [edk2-devel] [PATCH V2] MdePkg: Add Serial Terminal Device Type
> Guid
> 
> Add definition of EFI_SERIAL_TERMINAL_DEVICE_TYPE_GUID.
> It was miseed in "Extend SERIAL_IO with DeviceTypeGuid" patch.
> (UEFI 2.8, mantis 1832)
> 
> Signed-off-by: Oleksiy Yakovlev 
> ---
>  MdePkg/Include/Protocol/SerialIo.h | 6 ++
>  MdePkg/MdePkg.dec  | 3 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/MdePkg/Include/Protocol/SerialIo.h
> b/MdePkg/Include/Protocol/SerialIo.h
> index e2e0c61..16a865b 100644
> --- a/MdePkg/Include/Protocol/SerialIo.h
> +++ b/MdePkg/Include/Protocol/SerialIo.h
> @@ -17,6 +17,11 @@
>  0xBB25CF6F, 0xF1D4, 0x11D2, {0x9A, 0x0C, 0x00, 0x90, 0x27, 0x3F, 0xC1,
> 0xFD } \
>}
> 
> +#define EFI_SERIAL_TERMINAL_DEVICE_TYPE_GUID \
> +  { \
> +0X6AD9A60F, 0X5815, 0X4C7C, { 0X8A, 0X10, 0X50, 0X53, 0XD2, 0XBF,
> +0X7A, 0X1B } \
> +  }
> +
>  ///
>  /// Protocol GUID defined in EFI1.1.
>  ///
> @@ -299,5 +304,6 @@ struct _EFI_SERIAL_IO_PROTOCOL {  };
> 
>  extern EFI_GUID gEfiSerialIoProtocolGuid;
> +extern EFI_GUID gEfiSerialTerminalDeviceTypeGuid;
> 
>  #endif
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> d03fc5b..fc4dae2 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -670,6 +670,9 @@
>## Include/Guid/RtPropertiesTable.h
>gEfiRtPropertiesTableGuid  = { 0xeb66918a, 0x7eef, 0x402a, { 0x84, 
> 0x2e,
> 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9 }}
> 
> +  ## Include/Protocol/SerilaIo.h
> +  gEfiSerialTerminalDeviceTypeGuid = { 0x6AD9A60F, 0x5815, 0x4C7C, {
> + 0x08A, 0x10, 0x50, 0x53, 0xD2, 0xBF, 0x7A, 0x1B }}
> +
>#
># GUID defined in PI1.0
>#
> --
> 2.9.0.windows.1
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI).  This communication is intended
> to be read only by the individual or entity to whom it is addressed or by 
> their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited.  Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61765): https://edk2.groups.io/g/devel/message/61765
Mute This Topic: https://groups.io/mt/75085251/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 1/1] IntelFsp2Pkg/Tools/GenCfgOpt.py: Fix a bug about parse macro

2020-06-28 Thread Tan, Ming
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2827

Fix a bug about parse the macro value which use another macro.

Use the following example to verify:
[Define]
  DEFINE M1 = V1
  DEFINE M2 = $(M1)/V2

  !include $(M2)/pcd.dsc

The old code will failed parse M2 and cause following error:
Traceback (most recent call last):
  File "Edk2\IntelFsp2Pkg\Tools\GenCfgOpt.py", line 1550, in 
sys.exit(Main())
  File "Edk2\IntelFsp2Pkg\Tools\GenCfgOpt.py", line 1513, in Main
if GenCfgOpt.ParseDscFile(DscFile, FvDir) != 0:
  File "Edk2\IntelFsp2Pkg\Tools\GenCfgOpt.py", line 533, in ParseDscFile
NewDscLines = IncludeDsc.readlines()
ValueError: I/O operation on closed file.

The tool should support the value use another macro, and expand it.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Star Zeng 
Signed-off-by: Ming Tan 
---
 IntelFsp2Pkg/Tools/GenCfgOpt.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
index e6c15108f5c5..e9de128e5055 100644
--- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
+++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
@@ -546,11 +546,11 @@ EndList
 #DEFINE FSP_T_UPD_TOOL_GUID = 
34686CA3-34F9-4901-B82A-BA630F0714C6
 #DEFINE FSP_M_UPD_TOOL_GUID = 
39A250DB-E465-4DD1-A2AC-E2BD3C0E2385
 #DEFINE FSP_S_UPD_TOOL_GUID = 
CAE3605B-5B34-4C85-B3D7-27D54273C40F
-Match = re.match("^\s*(?:DEFINE\s+)*(\w+)\s*=\s*([-.\w]+)", 
DscLine)
+Match = 
re.match("^\s*(?:DEFINE\s+)*(\w+)\s*=\s*([/$()-.\w]+)", DscLine)
 if Match:
-self._MacroDict[Match.group(1)] = Match.group(2)
+self._MacroDict[Match.group(1)] = 
self.ExpandMacros(Match.group(2))
 if self.Debug:
-print ("INFO : DEFINE %s = [ %s ]" % (Match.group(1), 
Match.group(2)))
+print ("INFO : DEFINE %s = [ %s ]" % (Match.group(1), 
self.ExpandMacros(Match.group(2
 elif IsPcdSect:
 #gSiPkgTokenSpaceGuid.PcdTxtEnable|FALSE
 #gSiPkgTokenSpaceGuid.PcdOverclockEnable|TRUE
-- 
2.24.0.windows.2


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61764): https://edk2.groups.io/g/devel/message/61764
Mute This Topic: https://groups.io/mt/75166595/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH v1 1/1] MdePkg : UefiFileHandleLib: fix buffer overrun in FileHandleReadLine()

2020-06-28 Thread Zhiguang Liu
Hi Vladimir
Thanks for catching this bug. And I agree with you about your code change.
One little problem is that we always define the variable in the beginning of 
the function.
Please fix the little issue and I will give my RB.

Thanks
Zhiguang

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Vladimir
> Olovyannikov via groups.io
> Sent: Friday, June 5, 2020 8:18 AM
> To: devel@edk2.groups.io
> Cc: Vladimir Olovyannikov ; Kinney,
> Michael D ; Gao, Liming
> 
> Subject: [edk2-devel] [PATCH v1 1/1] MdePkg : UefiFileHandleLib: fix buffer
> overrun in FileHandleReadLine()
> 
> If the size of the supplied buffer in FileHandleReadLine(), module
> UefiFileHandleLib.c, was not 0, but was not enough to fit in
> the line, the size is increased, and then the Buffer of the new
> size is zeroed. This size is always larger than the supplied buffer size,
> causing supplied buffer overrun. Fix the issue by using the
> supplied buffer size in ZeroMem().
> 
> Signed-off-by: Vladimir Olovyannikov
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> ---
>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> index 28e28e5f67d5..4bc9fabb6c74 100644
> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> @@ -1039,10 +1039,13 @@ FileHandleReadLine(
>// if we ran out of space tell when...
> 
>//
> 
>if ((CountSoFar+1-CrCount)*sizeof(CHAR16) > *Size){
> 
> +UINTN OldSize;
> 
> +
> 
> +OldSize = *Size;
> 
>  *Size = (CountSoFar+1-CrCount)*sizeof(CHAR16);
> 
>  if (!Truncate) {
> 
> -  if (Buffer != NULL && *Size != 0) {
> 
> -ZeroMem(Buffer, *Size);
> 
> +  if (Buffer != NULL && OldSize != 0) {
> 
> +ZeroMem(Buffer, OldSize);
> 
>}
> 
>FileHandleSetPosition(Handle, OriginalFilePosition);
> 
>return (EFI_BUFFER_TOO_SMALL);
> 
> --
> 2.26.2.266.ge870325ee8
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#60736): https://edk2.groups.io/g/devel/message/60736
> Mute This Topic: https://groups.io/mt/74683735/1779286
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [zhiguang@intel.com]
> -=-=-=-=-=-=


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61763): https://edk2.groups.io/g/devel/message/61763
Mute This Topic: https://groups.io/mt/74683735/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-