[Patch] ShellPkg/AcpiView: Fix IA32 link error

[Patch] ShellPkg/AcpiView: Fix IA32 link error

[Michael D Kinney]

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

Update local variable in ParseAcpiSlot() to be UINT32
instead of UINT64 to avoid 64-bit multiply operation
in the SLIT_ELEMENT() macro.

Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index 1f9dac66ee..af85c9aa1c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
@@ -57,8 +57,8 @@ ParseAcpiSlit (
   )
 {
   UINT32 Offset;
-  UINT64 Count;
-  UINT64 Index;
+  UINT32 Count;
+  UINT32 Index;
   UINT64 LocalityCount;
   UINT8* LocalityPtr;
   CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
-- 
2.21.0.windows.1

Re: [Patch] ShellPkg/AcpiView: Fix IA32 link error

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, July 10, 2019 3:35 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>
> Subject: [Patch] ShellPkg/AcpiView: Fix IA32 link error
> Importance: High
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1970
> 
> Update local variable in ParseAcpiSlot() to be UINT32
> instead of UINT64 to avoid 64-bit multiply operation
> in the SLIT_ELEMENT() macro.
> 
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c     | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> index 1f9dac66ee..af85c9aa1c 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
> @@ -57,8 +57,8 @@ ParseAcpiSlit (
>    )
>  {
>    UINT32 Offset;
> -  UINT64 Count;
> -  UINT64 Index;
> +  UINT32 Count;
> +  UINT32 Index;
>    UINT64 LocalityCount;
>    UINT8* LocalityPtr;
>    CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
> --
> 2.21.0.windows.1

Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

[Sami Mujawar]

Hi Mike,

Since LocalityCount is 64-bit wide the SLIT validation code could possibly end up in an infinite loop. I am not aware of a platform that has a large enough LocalityCount to hit this condition. However, would it be good to have a check that limits the validation to MAX_UINT32?

e.g. Something like
if  (LocalityCount < MAX_UINT32) {
  // Validate
  for (Count = 0; Count < LocalityCount; Count++) {
    for (Index = 0; Index < LocalityCount; Index++) {
  ...
} else {
  Print (L"INFO: Skipping validation of System Localities as locality count is > MAX_UINT32\n");
}

Regards,

Sami Mujawar

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney via Groups.Io
Sent: 10 July 2019 11:35 PM
To: devel@edk2.groups.io
Cc: Jaben Carsey <jaben.carsey@intel.com>; Ray Ni <ray.ni@intel.com>; Zhichao Gao <zhichao.gao@intel.com>
Subject: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

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

Update local variable in ParseAcpiSlot() to be UINT32 instead of UINT64 to avoid 64-bit multiply operation in the SLIT_ELEMENT() macro.

Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
index 1f9dac66ee..af85c9aa1c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitPars
+++ er.c
@@ -57,8 +57,8 @@ ParseAcpiSlit (
   )
 {
   UINT32 Offset;
-  UINT64 Count;
-  UINT64 Index;
+  UINT32 Count;
+  UINT32 Index;
   UINT64 LocalityCount;
   UINT8* LocalityPtr;
   CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
--
2.21.0.windows.1




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.

Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

[Michael D Kinney]

Hi Sami,

I agree with your feedback.  I saw that there was a larger
patch set for the ShellPkg, so I let that complete before
returning to this topic.

The reason that I noticed this issue in the first place is
when I added the acpiview command to a platform and there
was an IA32 build failure.  It would be better if the ShellPkg
build caught this issue.  Adding the acpiview command to
the standard shell build adds 50K to an uncompressed DEBUG
IA32 build.

    869,312 Shell_7C04A583-9E3E-4f1c-AD65-E05268D0B4D1.efi
    920,928 Shell_EA4BB293-2D7F-4456-A681-1F22F42CD0BC.efi

Should acpiview be added to the standard ShellPkg build,
or should I add an extra build of the Shell to the 
ShellPkg DSC file with a different GUID to make sure
the shell builds when all NULL libs are included without
any !if statements.  For example:

  ShellPkg/Application/Shell/Shell.inf {
   <Defines>
      FILE_GUID = EA4BB293-2D7F-4456-A681-1F22F42CD0BC
    <PcdsFixedAtBuild>
      gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
    <LibraryClasses>
      NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
      NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
  }

Thanks,

Mike

> -----Original Message-----
> From: Sami Mujawar [mailto:Sami.Mujawar@arm.com]
> Sent: Thursday, July 11, 2019 1:24 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>
> Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView:
> Fix IA32 link error
> 
> Hi Mike,
> 
> Since LocalityCount is 64-bit wide the SLIT validation
> code could possibly end up in an infinite loop. I am
> not aware of a platform that has a large enough
> LocalityCount to hit this condition. However, would it
> be good to have a check that limits the validation to
> MAX_UINT32?
> 
> e.g. Something like
> if  (LocalityCount < MAX_UINT32) {
>   // Validate
>   for (Count = 0; Count < LocalityCount; Count++) {
>     for (Index = 0; Index < LocalityCount; Index++) {
>   ...
> } else {
>   Print (L"INFO: Skipping validation of System
> Localities as locality count is > MAX_UINT32\n"); }
> 
> Regards,
> 
> Sami Mujawar
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Michael D Kinney via Groups.Io
> Sent: 10 July 2019 11:35 PM
> To: devel@edk2.groups.io
> Cc: Jaben Carsey <jaben.carsey@intel.com>; Ray Ni
> <ray.ni@intel.com>; Zhichao Gao <zhichao.gao@intel.com>
> Subject: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix
> IA32 link error
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1970
> 
> Update local variable in ParseAcpiSlot() to be UINT32
> instead of UINT64 to avoid 64-bit multiply operation in
> the SLIT_ELEMENT() macro.
> 
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> ---
> 
> .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser
> .c     | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> Slit/SlitParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> Slit/SlitParser.c
> index 1f9dac66ee..af85c9aa1c 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> Slit/SlitParser.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> Slit/SlitPars
> +++ er.c
> @@ -57,8 +57,8 @@ ParseAcpiSlit (
>    )
>  {
>    UINT32 Offset;
> -  UINT64 Count;
> -  UINT64 Index;
> +  UINT32 Count;
> +  UINT32 Index;
>    UINT64 LocalityCount;
>    UINT8* LocalityPtr;
>    CHAR16 Buffer[80];  // Used for AsciiName param of
> ParseAcpi
> --
> 2.21.0.windows.1
> 
> 
> 
> 
> 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.

Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

[Carsey, Jaben]

I think the shell DSC be updated to catch build errors in the NULL libs. 

I am good with Mike's proposed solution below, but not very concerned with the method to catch the error.

Thanks
-Jaben


> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, August 01, 2019 1:30 PM
> To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>
> Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error
> 
> Hi Sami,
> 
> I agree with your feedback.  I saw that there was a larger
> patch set for the ShellPkg, so I let that complete before
> returning to this topic.
> 
> The reason that I noticed this issue in the first place is
> when I added the acpiview command to a platform and there
> was an IA32 build failure.  It would be better if the ShellPkg
> build caught this issue.  Adding the acpiview command to
> the standard shell build adds 50K to an uncompressed DEBUG
> IA32 build.
> 
>     869,312 Shell_7C04A583-9E3E-4f1c-AD65-E05268D0B4D1.efi
>     920,928 Shell_EA4BB293-2D7F-4456-A681-1F22F42CD0BC.efi
> 
> Should acpiview be added to the standard ShellPkg build,
> or should I add an extra build of the Shell to the
> ShellPkg DSC file with a different GUID to make sure
> the shell builds when all NULL libs are included without
> any !if statements.  For example:
> 
>   ShellPkg/Application/Shell/Shell.inf {
>    <Defines>
>       FILE_GUID = EA4BB293-2D7F-4456-A681-1F22F42CD0BC
>     <PcdsFixedAtBuild>
>       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>     <LibraryClasses>
> 
> NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comma
> ndsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Comma
> ndsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Comma
> ndsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Com
> mandsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1Com
> mandsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Com
> mandsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1
> CommandsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2
> CommandsLib.inf
> 
> NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCo
> mmandLib.inf
>   }
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: Sami Mujawar [mailto:Sami.Mujawar@arm.com]
> > Sent: Thursday, July 11, 2019 1:24 AM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Gao, Zhichao
> > <zhichao.gao@intel.com>
> > Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView:
> > Fix IA32 link error
> >
> > Hi Mike,
> >
> > Since LocalityCount is 64-bit wide the SLIT validation
> > code could possibly end up in an infinite loop. I am
> > not aware of a platform that has a large enough
> > LocalityCount to hit this condition. However, would it
> > be good to have a check that limits the validation to
> > MAX_UINT32?
> >
> > e.g. Something like
> > if  (LocalityCount < MAX_UINT32) {
> >   // Validate
> >   for (Count = 0; Count < LocalityCount; Count++) {
> >     for (Index = 0; Index < LocalityCount; Index++) {
> >   ...
> > } else {
> >   Print (L"INFO: Skipping validation of System
> > Localities as locality count is > MAX_UINT32\n"); }
> >
> > Regards,
> >
> > Sami Mujawar
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> > Behalf Of Michael D Kinney via Groups.Io
> > Sent: 10 July 2019 11:35 PM
> > To: devel@edk2.groups.io
> > Cc: Jaben Carsey <jaben.carsey@intel.com>; Ray Ni
> > <ray.ni@intel.com>; Zhichao Gao <zhichao.gao@intel.com>
> > Subject: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix
> > IA32 link error
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1970
> >
> > Update local variable in ParseAcpiSlot() to be UINT32
> > instead of UINT64 to avoid 64-bit multiply operation in
> > the SLIT_ELEMENT() macro.
> >
> > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: Michael D Kinney
> > <michael.d.kinney@intel.com>
> > ---
> >
> > .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser
> > .c     | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitParser.c
> > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitParser.c
> > index 1f9dac66ee..af85c9aa1c 100644
> > ---
> > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitParser.c
> > +++
> > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitPars
> > +++ er.c
> > @@ -57,8 +57,8 @@ ParseAcpiSlit (
> >    )
> >  {
> >    UINT32 Offset;
> > -  UINT64 Count;
> > -  UINT64 Index;
> > +  UINT32 Count;
> > +  UINT32 Index;
> >    UINT64 LocalityCount;
> >    UINT8* LocalityPtr;
> >    CHAR16 Buffer[80];  // Used for AsciiName param of
> > ParseAcpi
> > --
> > 2.21.0.windows.1
> >
> >
> > 
> >
> > 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.

Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

[Sami Mujawar]

Hi Mike, All,

I agree it will be good to add acpiview to the standard ShellPkg build.

Regards,

Sami Mujawar

-----Original Message-----
From: Carsey, Jaben <jaben.carsey@intel.com>
Sent: 01 August 2019 09:46 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error

I think the shell DSC be updated to catch build errors in the NULL libs.

I am good with Mike's proposed solution below, but not very concerned with the method to catch the error.

Thanks
-Jaben


> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, August 01, 2019 1:30 PM
> To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link
> error
>
> Hi Sami,
>
> I agree with your feedback.  I saw that there was a larger patch set
> for the ShellPkg, so I let that complete before returning to this
> topic.
>
> The reason that I noticed this issue in the first place is when I
> added the acpiview command to a platform and there was an IA32 build
> failure.  It would be better if the ShellPkg build caught this issue.
> Adding the acpiview command to the standard shell build adds 50K to an
> uncompressed DEBUG
> IA32 build.
>
>     869,312 Shell_7C04A583-9E3E-4f1c-AD65-E05268D0B4D1.efi
>     920,928 Shell_EA4BB293-2D7F-4456-A681-1F22F42CD0BC.efi
>
> Should acpiview be added to the standard ShellPkg build, or should I
> add an extra build of the Shell to the ShellPkg DSC file with a
> different GUID to make sure the shell builds when all NULL libs are
> included without any !if statements.  For example:
>
>   ShellPkg/Application/Shell/Shell.inf {
>    <Defines>
>       FILE_GUID = EA4BB293-2D7F-4456-A681-1F22F42CD0BC
>     <PcdsFixedAtBuild>
>       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>     <LibraryClasses>
>
> NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comma
> ndsLib.inf
>
> NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Comma
> ndsLib.inf
>
> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Comma
> ndsLib.inf
>
> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Com
> mandsLib.inf
>
> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1Co
> NULL|m
> mandsLib.inf
>
> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Com
> mandsLib.inf
>
> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1
> CommandsLib.inf
>
> NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2
> CommandsLib.inf
>
> NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCo
> mmandLib.inf
>   }
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Sami Mujawar [mailto:Sami.Mujawar@arm.com]
> > Sent: Thursday, July 11, 2019 1:24 AM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> > Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView:
> > Fix IA32 link error
> >
> > Hi Mike,
> >
> > Since LocalityCount is 64-bit wide the SLIT validation code could
> > possibly end up in an infinite loop. I am not aware of a platform
> > that has a large enough LocalityCount to hit this condition.
> > However, would it be good to have a check that limits the validation
> > to MAX_UINT32?
> >
> > e.g. Something like
> > if  (LocalityCount < MAX_UINT32) {
> >   // Validate
> >   for (Count = 0; Count < LocalityCount; Count++) {
> >     for (Index = 0; Index < LocalityCount; Index++) {
> >   ...
> > } else {
> >   Print (L"INFO: Skipping validation of System Localities as
> > locality count is > MAX_UINT32\n"); }
> >
> > Regards,
> >
> > Sami Mujawar
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > Michael D Kinney via Groups.Io
> > Sent: 10 July 2019 11:35 PM
> > To: devel@edk2.groups.io
> > Cc: Jaben Carsey <jaben.carsey@intel.com>; Ray Ni
> > <ray.ni@intel.com>; Zhichao Gao <zhichao.gao@intel.com>
> > Subject: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix
> > IA32 link error
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1970
> >
> > Update local variable in ParseAcpiSlot() to be UINT32 instead of
> > UINT64 to avoid 64-bit multiply operation in the SLIT_ELEMENT()
> > macro.
> >
> > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Signed-off-by: Michael D Kinney
> > <michael.d.kinney@intel.com>
> > ---
> >
> > .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser
> > .c     | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitParser.c
> > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitParser.c
> > index 1f9dac66ee..af85c9aa1c 100644
> > ---
> > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitParser.c
> > +++
> > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/
> > Slit/SlitPars
> > +++ er.c
> > @@ -57,8 +57,8 @@ ParseAcpiSlit (
> >    )
> >  {
> >    UINT32 Offset;
> > -  UINT64 Count;
> > -  UINT64 Index;
> > +  UINT32 Count;
> > +  UINT32 Index;
> >    UINT64 LocalityCount;
> >    UINT8* LocalityPtr;
> >    CHAR16 Buffer[80];  // Used for AsciiName param of ParseAcpi
> > --
> > 2.21.0.windows.1
> >
> >
> > 
> >
> > 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.
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.