Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-06-12 Thread Gao, Zhichao 
Thanks for the interpret. Seems I read the old version spec. Now it is clear 
for me.

Reviewed-by: Zhichao Gao 

> -Original Message-
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Wednesday, June 12, 2019 6:44 PM
> To: Gao, Zhichao ; devel@edk2.groups.io
> Cc: Sami Mujawar ; Carsey, Jaben
> ; Ni, Ray ; nd 
> Subject: Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update
> for MADT parser
> 
> Hi Zhichao,
> 
> Please find my comments inline below. They are marked with [Krzysztof]
> 
> Kind regards,
> 
> Krzysztof
> 
> -Original Message-
> From: Gao, Zhichao 
> Sent: Monday, June 10, 2019 2:08
> To: Krzysztof Koch ; devel@edk2.groups.io
> Cc: Sami Mujawar ; Carsey, Jaben
> ; Ni, Ray ; Matteo Carlini
> ; Stephanie Hughes-Fitt  f...@arm.com>; nd 
> Subject: RE: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT
> parser
> 
> Sorry for late update.
> 
> > -Original Message-
> > From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> > Sent: Friday, June 7, 2019 4:48 PM
> > To: devel@edk2.groups.io
> > Cc: sami.muja...@arm.com; Carsey, Jaben ; Ni,
> > Ray ; Gao, Zhichao ;
> > matteo.carl...@arm.com; stephanie.hughes-f...@arm.com; n...@arm.com
> > Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT
> > parser
> >
> > The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field
> > as part of the GICC structure.
> >
> > Update the MADT parser to decode this field and validate the interrupt
> > ID used.
> >
> > References:
> > - ACPI 6.3 Specification - January 2019
> > - Arm Generic Interrupt Controller Architecture Specification,
> >   GIC architecture version 3 and version 4, issue E
> > - Arm Server Base System Architecture 5.0
> >
> > Signed-off-by: Krzysztof Koch 
> > ---
> >
> > Changes can be seen at:
> > https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2
> >
> > Notes:
> > v2:
> > - Add include sandwich in MadtParser.h [Zhichao]
> > - Add references to specifications in commit message [Zhichao]
> >
> > v1:
> > - Decode and validate SPE Overflow Interrupt field [Krzysztof]
> >
> >
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> > c | 86 ++--
> >
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> > h | 40 +
> >  2 files changed, 118 insertions(+), 8 deletions(-)
> >
> > diff --git
> >
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> > er.c
> >
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> > er.c
> > index
> >
> a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36f
> > c63f1e4ab866f 100644
> > ---
> >
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> > er.c
> > +++
> >
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> > +++ er.c
> > @@ -1,17 +1,21 @@
> >  /** @file
> >MADT table parser
> >
> > -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> > +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> >SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >@par Reference(s):
> > -- ACPI 6.2 Specification - Errata A, September 2017
> > +- ACPI 6.3 Specification - January 2019
> > +- Arm Generic Interrupt Controller Architecture Specification,
> > +  GIC architecture version 3 and version 4, issue E
> > +- Arm Server Base System Architecture 5.0
> >  **/
> >
> >  #include 
> >  #include 
> >  #include "AcpiParser.h"
> >  #include "AcpiTableParser.h"
> > +#include "MadtParser.h"
> >
> >  // Local Variables
> >  STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21
> @@
> > ValidateGICDSystemVectorBase (
> >IN VOID*  Context
> >);
> >
> > +/**
> > +  This function validates the SPE Overflow Interrupt in the GICC.
> > +
> > +  @param [in] Ptr Pointer to the start of the field data.
> > +  @param [in] Context Pointer to context specific information e.g. this
> > +  could be a pointer to the ACPI table header.
> > +**/
> > +STATIC
> > +VOID
> > +EFIAPI
> > +ValidateSpeOverflowInterrupt (
> > +  IN UINT8* Ptr,
> > +  IN VOID*  Context
>

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-06-12 Thread Sami Mujawar 
Reviewed-by: Sami Mujawar 

-Original Message-
From: Krzysztof Koch  
Sent: 12 June 2019 11:44 AM
To: Gao, Zhichao ; devel@edk2.groups.io
Cc: Sami Mujawar ; Carsey, Jaben 
; Ni, Ray ; nd 
Subject: RE: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

Hi Zhichao,

Please find my comments inline below. They are marked with [Krzysztof]

Kind regards,

Krzysztof

-Original Message-
From: Gao, Zhichao 
Sent: Monday, June 10, 2019 2:08
To: Krzysztof Koch ; devel@edk2.groups.io
Cc: Sami Mujawar ; Carsey, Jaben 
; Ni, Ray ; Matteo Carlini 
; Stephanie Hughes-Fitt 
; nd 
Subject: RE: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

Sorry for late update.

> -Original Message-
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Friday, June 7, 2019 4:48 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; Carsey, Jaben ; Ni, 
> Ray ; Gao, Zhichao ; 
> matteo.carl...@arm.com; stephanie.hughes-f...@arm.com; n...@arm.com
> Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT 
> parser
> 
> The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field 
> as part of the GICC structure.
> 
> Update the MADT parser to decode this field and validate the interrupt 
> ID used.
> 
> References:
> - ACPI 6.3 Specification - January 2019
> - Arm Generic Interrupt Controller Architecture Specification,
>   GIC architecture version 3 and version 4, issue E
> - Arm Server Base System Architecture 5.0
> 
> Signed-off-by: Krzysztof Koch 
> ---
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2
> 
> Notes:
> v2:
> - Add include sandwich in MadtParser.h [Zhichao]
> - Add references to specifications in commit message [Zhichao]
> 
> v1:
> - Decode and validate SPE Overflow Interrupt field [Krzysztof]
> 
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 86 ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> h | 40 +
>  2 files changed, 118 insertions(+), 8 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> index
> a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36f
> c63f1e4ab866f 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> +++ er.c
> @@ -1,17 +1,21 @@
>  /** @file
>MADT table parser
> 
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>@par Reference(s):
> -- ACPI 6.2 Specification - Errata A, September 2017
> +- ACPI 6.3 Specification - January 2019
> +- Arm Generic Interrupt Controller Architecture Specification,
> +  GIC architecture version 3 and version 4, issue E
> +- Arm Server Base System Architecture 5.0
>  **/
> 
>  #include 
>  #include 
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "MadtParser.h"
> 
>  // Local Variables
>  STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@ 
> ValidateGICDSystemVectorBase (
>IN VOID*  Context
>);
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  );
> +
>  /**
>An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
>  **/
> @@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
>{L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
>{L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
> NULL},
> -  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
> +  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},  {L"SPE 
> + overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
> +ValidateSpeOverflowInterrupt, NULL}
>  };
> 
>  /**
> @@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
>}
>  }
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +  UINT16 SpeOverflowInterrupt;
> +
> +  SpeOverflowInterrupt = *(UINT16*)Ptr;
> +
> +  // SPE not supported by this processor  if (SpeOverflowIn

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-06-12 Thread Krzysztof Koch 
Hi Zhichao,

Please find my comments inline below. They are marked with [Krzysztof]

Kind regards,

Krzysztof

-Original Message-
From: Gao, Zhichao  
Sent: Monday, June 10, 2019 2:08
To: Krzysztof Koch ; devel@edk2.groups.io
Cc: Sami Mujawar ; Carsey, Jaben 
; Ni, Ray ; Matteo Carlini 
; Stephanie Hughes-Fitt 
; nd 
Subject: RE: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

Sorry for late update.

> -Original Message-
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Friday, June 7, 2019 4:48 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; Carsey, Jaben ; Ni, 
> Ray ; Gao, Zhichao ; 
> matteo.carl...@arm.com; stephanie.hughes-f...@arm.com; n...@arm.com
> Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT 
> parser
> 
> The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field 
> as part of the GICC structure.
> 
> Update the MADT parser to decode this field and validate the interrupt 
> ID used.
> 
> References:
> - ACPI 6.3 Specification - January 2019
> - Arm Generic Interrupt Controller Architecture Specification,
>   GIC architecture version 3 and version 4, issue E
> - Arm Server Base System Architecture 5.0
> 
> Signed-off-by: Krzysztof Koch 
> ---
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2
> 
> Notes:
> v2:
> - Add include sandwich in MadtParser.h [Zhichao]
> - Add references to specifications in commit message [Zhichao]
> 
> v1:
> - Decode and validate SPE Overflow Interrupt field [Krzysztof]
> 
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 86 ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> h | 40 +
>  2 files changed, 118 insertions(+), 8 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> index
> a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36f
> c63f1e4ab866f 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> +++ er.c
> @@ -1,17 +1,21 @@
>  /** @file
>MADT table parser
> 
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>@par Reference(s):
> -- ACPI 6.2 Specification - Errata A, September 2017
> +- ACPI 6.3 Specification - January 2019
> +- Arm Generic Interrupt Controller Architecture Specification,
> +  GIC architecture version 3 and version 4, issue E
> +- Arm Server Base System Architecture 5.0
>  **/
> 
>  #include 
>  #include 
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "MadtParser.h"
> 
>  // Local Variables
>  STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@ 
> ValidateGICDSystemVectorBase (
>IN VOID*  Context
>);
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  );
> +
>  /**
>An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
>  **/
> @@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
>{L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
>{L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
> NULL},
> -  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
> +  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},  {L"SPE 
> + overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
> +ValidateSpeOverflowInterrupt, NULL}
>  };
> 
>  /**
> @@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
>}
>  }
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +  UINT16 SpeOverflowInterrupt;
> +
> +  SpeOverflowInterrupt = *(UINT16*)Ptr;
> +
> +  // SPE not supported by this processor  if (SpeOverflowInterrupt ==
> + 0) {
> +return;
> +  }
> +
> +  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
> +  ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
> +   (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
> +  (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
>

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-06-09 Thread Gao, Zhichao 
Sorry for late update.

> -Original Message-
> From: Krzysztof Koch [mailto:krzysztof.k...@arm.com]
> Sent: Friday, June 7, 2019 4:48 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; Carsey, Jaben ; Ni,
> Ray ; Gao, Zhichao ;
> matteo.carl...@arm.com; stephanie.hughes-f...@arm.com; n...@arm.com
> Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser
> 
> The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field as part
> of the GICC structure.
> 
> Update the MADT parser to decode this field and validate the interrupt ID
> used.
> 
> References:
> - ACPI 6.3 Specification - January 2019
> - Arm Generic Interrupt Controller Architecture Specification,
>   GIC architecture version 3 and version 4, issue E
> - Arm Server Base System Architecture 5.0
> 
> Signed-off-by: Krzysztof Koch 
> ---
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2
> 
> Notes:
> v2:
> - Add include sandwich in MadtParser.h [Zhichao]
> - Add references to specifications in commit message [Zhichao]
> 
> v1:
> - Decode and validate SPE Overflow Interrupt field [Krzysztof]
> 
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 86 ++--
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> h | 40 +
>  2 files changed, 118 insertions(+), 8 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> index
> a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36f
> c63f1e4ab866f 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> er.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
> +++ er.c
> @@ -1,17 +1,21 @@
>  /** @file
>MADT table parser
> 
> -  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
>SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>@par Reference(s):
> -- ACPI 6.2 Specification - Errata A, September 2017
> +- ACPI 6.3 Specification - January 2019
> +- Arm Generic Interrupt Controller Architecture Specification,
> +  GIC architecture version 3 and version 4, issue E
> +- Arm Server Base System Architecture 5.0
>  **/
> 
>  #include 
>  #include 
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
> +#include "MadtParser.h"
> 
>  // Local Variables
>  STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@
> ValidateGICDSystemVectorBase (
>IN VOID*  Context
>);
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  );
> +
>  /**
>An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
>  **/
> @@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
>{L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
>{L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
> NULL},
> -  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
> +  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},  {L"SPE
> + overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
> +ValidateSpeOverflowInterrupt, NULL}
>  };
> 
>  /**
> @@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
>}
>  }
> 
> +/**
> +  This function validates the SPE Overflow Interrupt in the GICC.
> +
> +  @param [in] Ptr Pointer to the start of the field data.
> +  @param [in] Context Pointer to context specific information e.g. this
> +  could be a pointer to the ACPI table header.
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ValidateSpeOverflowInterrupt (
> +  IN UINT8* Ptr,
> +  IN VOID*  Context
> +  )
> +{
> +  UINT16 SpeOverflowInterrupt;
> +
> +  SpeOverflowInterrupt = *(UINT16*)Ptr;
> +
> +  // SPE not supported by this processor  if (SpeOverflowInterrupt ==
> + 0) {
> +return;
> +  }
> +
> +  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
> +  ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
> +   (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
> +  (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
> +IncrementErrorCount ();
> +Print (
> +  L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI 
> ID "
> +L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
> +  SpeOverflowInterrupt,
> +  ARM_PPI_ID_MIN,
> +  ARM_PPI_ID_MAX,
> +  ARM_PPI_ID_EXTENDED_MIN,
> +  ARM_PPI_ID_EXTENDED_MAX
> +);
> +  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
> +

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-06-07 Thread Sami Mujawar 
Reviewed-by: Sami Mujawar 


-Original Message-
From: Krzysztof Koch  
Sent: 07 June 2019 09:48 AM
To: devel@edk2.groups.io
Cc: Sami Mujawar ; jaben.car...@intel.com; 
ray...@intel.com; zhichao@intel.com; Matteo Carlini 
; Stephanie Hughes-Fitt 
; nd 
Subject: [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

The ACPI 6.3 specification introduces a 'SPE overflow Interrupt' field as part 
of the GICC structure.

Update the MADT parser to decode this field and validate the interrupt ID used.

References:
- ACPI 6.3 Specification - January 2019
- Arm Generic Interrupt Controller Architecture Specification,
  GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0

Signed-off-by: Krzysztof Koch 
---

Changes can be seen at: 
https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2

Notes:
v2:
- Add include sandwich in MadtParser.h [Zhichao]
- Add references to specifications in commit message [Zhichao]

v1:
- Decode and validate SPE Overflow Interrupt field [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 86 
++--  
ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 40 
+
 2 files changed, 118 insertions(+), 8 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 
a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36fc63f1e4ab866f
 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtPars
+++ er.c
@@ -1,17 +1,21 @@
 /** @file
   MADT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-- ACPI 6.2 Specification - Errata A, September 2017
+- ACPI 6.3 Specification - January 2019
+- Arm Generic Interrupt Controller Architecture Specification,
+  GIC architecture version 3 and version 4, issue E
+- Arm Server Base System Architecture 5.0
 **/
 
 #include 
 #include 
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "MadtParser.h"
 
 // Local Variables
 STATIC CONST UINT8* MadtInterruptControllerType; @@ -33,6 +37,21 @@ 
ValidateGICDSystemVectorBase (
   IN VOID*  Context
   );
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  );
+
 /**
   An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
 **/
@@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
   {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
NULL},
-  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
+  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},  {L"SPE 
+ overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
+ValidateSpeOverflowInterrupt, NULL}
 };
 
 /**
@@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
   }
 }
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  UINT16 SpeOverflowInterrupt;
+
+  SpeOverflowInterrupt = *(UINT16*)Ptr;
+
+  // SPE not supported by this processor  if (SpeOverflowInterrupt == 
+ 0) {
+return;
+  }
+
+  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
+  ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
+   (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
+  (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
+IncrementErrorCount ();
+Print (
+  L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
+L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_MIN,
+  ARM_PPI_ID_MAX,
+  ARM_PPI_ID_EXTENDED_MIN,
+  ARM_PPI_ID_EXTENDED_MAX
+);
+  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
+IncrementWarningCount();
+Print (
+  L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
+L"Level 3 PPI ID assignment: %d.",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_PMBIRQ
+);
+  }
+}
+
 /**
   This function parses the ACPI MADT table.
   When trace is enabled this function parses the

Re: [edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-06-07 Thread Alexei . Fedorov 
Reviewed-by: Alexei Fedorov 

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

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

[edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-06-07 Thread Krzysztof Koch 
The ACPI 6.3 specification introduces a 'SPE overflow
Interrupt' field as part of the GICC structure.

Update the MADT parser to decode this field and validate
the interrupt ID used.

References:
- ACPI 6.3 Specification - January 2019
- Arm Generic Interrupt Controller Architecture Specification,
  GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0

Signed-off-by: Krzysztof Koch 
---

Changes can be seen at: 
https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2

Notes:
v2:
- Add include sandwich in MadtParser.h [Zhichao]
- Add references to specifications in commit message [Zhichao]

v1:
- Decode and validate SPE Overflow Interrupt field [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 86 
++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 40 
+
 2 files changed, 118 insertions(+), 8 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 
a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36fc63f1e4ab866f
 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -1,17 +1,21 @@
 /** @file
   MADT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-- ACPI 6.2 Specification - Errata A, September 2017
+- ACPI 6.3 Specification - January 2019
+- Arm Generic Interrupt Controller Architecture Specification,
+  GIC architecture version 3 and version 4, issue E
+- Arm Server Base System Architecture 5.0
 **/
 
 #include 
 #include 
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "MadtParser.h"
 
 // Local Variables
 STATIC CONST UINT8* MadtInterruptControllerType;
@@ -33,6 +37,21 @@ ValidateGICDSystemVectorBase (
   IN VOID*  Context
   );
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  );
+
 /**
   An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
 **/
@@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
   {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
NULL},
-  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
+  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},
+  {L"SPE overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
+ValidateSpeOverflowInterrupt, NULL}
 };
 
 /**
@@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
   }
 }
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  UINT16 SpeOverflowInterrupt;
+
+  SpeOverflowInterrupt = *(UINT16*)Ptr;
+
+  // SPE not supported by this processor
+  if (SpeOverflowInterrupt == 0) {
+return;
+  }
+
+  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
+  ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
+   (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
+  (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
+IncrementErrorCount ();
+Print (
+  L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
+L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_MIN,
+  ARM_PPI_ID_MAX,
+  ARM_PPI_ID_EXTENDED_MIN,
+  ARM_PPI_ID_EXTENDED_MAX
+);
+  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
+IncrementWarningCount();
+Print (
+  L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
+L"Level 3 PPI ID assignment: %d.",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_PMBIRQ
+);
+  }
+}
+
 /**
   This function parses the ACPI MADT table.
   When trace is enabled this function parses the MADT table and
@@ -233,7 +303,7 @@ ParseAcpiMadt (
 }
 
 switch (*MadtInterruptControllerType) {
-  case EFI_ACPI_6_2_GIC: {
+  case EFI_ACPI_6_3_GIC: {
 ParseAcpi (
   TRUE,
   2,
@@ -245,7 +315,7 @@ ParseAcpiMadt (
 break;
   }
 
-  case EFI_ACPI_6_2_GICD: {
+  case EFI_ACPI_6_3_GICD: {

[edk2-devel] [PATCH v2 1/1] ShellPkg: acpiview: ACPI 6.3 update for MADT parser

2019-05-30 Thread Krzysztof Koch 
The ACPI 6.3 specification introduces a 'SPE overflow
Interrupt' field as part of the GICC structure.

Update the MADT parser to decode this field and validate
the interrupt ID used.

References:
- ACPI 6.3 Specification - January 2019
- Arm Generic Interrupt Controller Architecture Specification,
  GIC architecture version 3 and version 4, issue E
- Arm Server Base System Architecture 5.0

Signed-off-by: Krzysztof Koch 
---

Changes can be seen at: 
https://github.com/KrzysztofKoch1/edk2/tree/477_acpiview_spe_v2

Notes:
v2:
- Add include sandwich in MadtParser.h [Zhichao]
- Add references to specifications in commit message [Zhichao]

v1:
- Decode and validate SPE Overflow Interrupt field [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 86 
++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.h | 40 
+
 2 files changed, 118 insertions(+), 8 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 
a1bf86ade5313f954a77b325c13394cfce133939..59c3df0cc8a080497b517baf36fc63f1e4ab866f
 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -1,17 +1,21 @@
 /** @file
   MADT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
-- ACPI 6.2 Specification - Errata A, September 2017
+- ACPI 6.3 Specification - January 2019
+- Arm Generic Interrupt Controller Architecture Specification,
+  GIC architecture version 3 and version 4, issue E
+- Arm Server Base System Architecture 5.0
 **/
 
 #include 
 #include 
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
+#include "MadtParser.h"
 
 // Local Variables
 STATIC CONST UINT8* MadtInterruptControllerType;
@@ -33,6 +37,21 @@ ValidateGICDSystemVectorBase (
   IN VOID*  Context
   );
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  );
+
 /**
   An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
 **/
@@ -56,7 +75,9 @@ STATIC CONST ACPI_PARSER GicCParser[] = {
   {L"MPIDR", 8, 68, L"0x%lx", NULL, NULL, NULL, NULL},
   {L"Processor Power Efficiency Class", 1, 76, L"0x%x", NULL, NULL, NULL,
NULL},
-  {L"Reserved", 3, 77, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
+  {L"Reserved", 1, 77, L"0x%x", NULL, NULL, NULL, NULL},
+  {L"SPE overflow Interrupt", 2, 78, L"0x%x", NULL, NULL,
+ValidateSpeOverflowInterrupt, NULL}
 };
 
 /**
@@ -160,6 +181,55 @@ ValidateGICDSystemVectorBase (
   }
 }
 
+/**
+  This function validates the SPE Overflow Interrupt in the GICC.
+
+  @param [in] Ptr Pointer to the start of the field data.
+  @param [in] Context Pointer to context specific information e.g. this
+  could be a pointer to the ACPI table header.
+**/
+STATIC
+VOID
+EFIAPI
+ValidateSpeOverflowInterrupt (
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  UINT16 SpeOverflowInterrupt;
+
+  SpeOverflowInterrupt = *(UINT16*)Ptr;
+
+  // SPE not supported by this processor
+  if (SpeOverflowInterrupt == 0) {
+return;
+  }
+
+  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
+  ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
+   (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
+  (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
+IncrementErrorCount ();
+Print (
+  L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
+L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_MIN,
+  ARM_PPI_ID_MAX,
+  ARM_PPI_ID_EXTENDED_MIN,
+  ARM_PPI_ID_EXTENDED_MAX
+);
+  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
+IncrementWarningCount();
+Print (
+  L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
+L"Level 3 PPI ID assignment: %d.",
+  SpeOverflowInterrupt,
+  ARM_PPI_ID_PMBIRQ
+);
+  }
+}
+
 /**
   This function parses the ACPI MADT table.
   When trace is enabled this function parses the MADT table and
@@ -233,7 +303,7 @@ ParseAcpiMadt (
 }
 
 switch (*MadtInterruptControllerType) {
-  case EFI_ACPI_6_2_GIC: {
+  case EFI_ACPI_6_3_GIC: {
 ParseAcpi (
   TRUE,
   2,
@@ -245,7 +315,7 @@ ParseAcpiMadt (
 break;
   }
 
-  case EFI_ACPI_6_2_GICD: {
+  case EFI_ACPI_6_3_GICD: {