Re: [PATCH] ShellPkg/pci: Support interpreting specific PCIE ext cap thru "-ec"

["Carsey, Jaben" <jaben.carsey@intel.com>]

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

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, January 23, 2017 10:43 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [PATCH] ShellPkg/pci: Support interpreting specific PCIE ext cap thru
> "-ec"
> Importance: High
> 
> The implementation was already there but through a private flag
> "-_e". The patch removes "-_e" support and add "-ec" support.
> Removing old "-_e" support makes the pci command more clean.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> ---
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c  | 61 +++++++++-------
> ------
>  .../UefiShellDebug1CommandsLib.uni                 | 15 +++---
>  2 files changed, 34 insertions(+), 42 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> index 9cc4f5c..99e6caf 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> @@ -2370,7 +2370,7 @@ PCI_CONFIG_SPACE  *mConfigSpace = NULL;
>  STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>    {L"-s", TypeValue},
>    {L"-i", TypeFlag},
> -  {L"-_e", TypeValue},
> +  {L"-ec", TypeValue},
>    {NULL, TypeMax}
>    };
> 
> @@ -2516,6 +2516,11 @@ ShellCommandRunPci (
>        ShellStatus = SHELL_INVALID_PARAMETER;
>        goto Done;
>      }
> +    if (ShellCommandLineGetFlag(Package, L"-ec") &&
> ShellCommandLineGetValue(Package, L"-ec") == NULL) {
> +      ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_VALUE),
> gShellDebug1HiiHandle,  L"pci", L"-ec");
> +      ShellStatus = SHELL_INVALID_PARAMETER;
> +      goto Done;
> +    }
>      if (ShellCommandLineGetFlag(Package, L"-s") &&
> ShellCommandLineGetValue(Package, L"-s") == NULL) {
>        ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_VALUE),
> gShellDebug1HiiHandle,  L"pci", L"-s");
>        ShellStatus = SHELL_INVALID_PARAMETER;
> @@ -2878,13 +2883,11 @@ ShellCommandRunPci (
>      // If "-i" appears in command line, interpret data in configuration space
>      //
>      if (ExplainData) {
> -      EnhancedDump = 0;
> -      if (ShellCommandLineGetFlag(Package, L"-_e")) {
> -        EnhancedDump = 0xFFFF;
> -        Temp = ShellCommandLineGetValue(Package, L"-_e");
> -        if (Temp != NULL) {
> -          EnhancedDump = (UINT16) ShellHexStrToUintn (Temp);
> -        }
> +      EnhancedDump = 0xFFFF;
> +      if (ShellCommandLineGetFlag(Package, L"-ec")) {
> +        Temp = ShellCommandLineGetValue(Package, L"-ec");
> +        ASSERT (Temp != NULL);
> +        EnhancedDump = (UINT16) ShellHexStrToUintn (Temp);
>        }
>        Status = PciExplainData (&ConfigSpace, Address, IoDev, EnhancedDump);
>      }
> @@ -5829,39 +5832,25 @@ PciExplainPciExpress (
>      return EFI_UNSUPPORTED;
>    }
> 
> -  if (EnhancedDump == 0) {
> +  ExtHdr = (PCI_EXP_EXT_HDR*)ExRegBuffer;
> +  while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0) {
>      //
> -    // Print the PciEx extend space in raw bytes ( 0xFF-0xFFF)
> +    // Process this item
>      //
> -    ShellPrintEx (-1, -1, L"\r\n%HStart dumping PCIex extended configuration
> space (0x100 - 0xFFF).%N\r\n\r\n");
> -
> -    DumpHex (
> -      2,
> -      EFI_PCIE_CAPABILITY_BASE_OFFSET,
> -      ExtendRegSize,
> -      (VOID *) (ExRegBuffer)
> -      );
> -  } else {
> -    ExtHdr = (PCI_EXP_EXT_HDR*)ExRegBuffer;
> -    while (ExtHdr->CapabilityId != 0 && ExtHdr->CapabilityVersion != 0) {
> +    if (EnhancedDump == 0xFFFF || EnhancedDump == ExtHdr->CapabilityId)
> {
>        //
> -      // Process this item
> +      // Print this item
>        //
> -      if (EnhancedDump == 0xFFFF || EnhancedDump == ExtHdr->CapabilityId)
> {
> -        //
> -        // Print this item
> -        //
> -        PrintPciExtendedCapabilityDetails((PCI_EXP_EXT_HDR*)ExRegBuffer,
> ExtHdr, &PciExpressCap);
> -      }
> +      PrintPciExtendedCapabilityDetails((PCI_EXP_EXT_HDR*)ExRegBuffer,
> ExtHdr, &PciExpressCap);
> +    }
> 
> -      //
> -      // Advance to the next item if it exists
> -      //
> -      if (ExtHdr->NextCapabilityOffset != 0) {
> -        ExtHdr = (PCI_EXP_EXT_HDR*)((UINT8*)ExRegBuffer + ExtHdr-
> >NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET);
> -      } else {
> -        break;
> -      }
> +    //
> +    // Advance to the next item if it exists
> +    //
> +    if (ExtHdr->NextCapabilityOffset != 0) {
> +      ExtHdr = (PCI_EXP_EXT_HDR*)((UINT8*)ExRegBuffer + ExtHdr-
> >NextCapabilityOffset - EFI_PCIE_CAPABILITY_BASE_OFFSET);
> +    } else {
> +      break;
>      }
>    }
>    SHELL_FREE_NON_NULL(ExRegBuffer);
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> index 06865a4..324e233f 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.uni
> @@ -895,17 +895,20 @@
>  #string STR_GET_HELP_PCI          #language en-US ""
>  ".TH pci 0 "Displays PCI device information."\r\n"
>  ".SH NAME\r\n"
> -"Displays a PCI device list or PCI function configuration space of a
> device.\r\n"
> +"Displays PCI device list or PCI function configuration space and PCIe
> extended\r\n"
> +"configuration space.\r\n"
>  ".SH SYNOPSIS\r\n"
>  " \r\n"
> -"PCI [Bus Dev [Func] [-s Seg] [-i]]\r\n"
> +"PCI [Bus Dev [Func] [-s Seg] [-i [-ec ID]]]\r\n"
>  ".SH OPTIONS\r\n"
>  " \r\n"
> -"  -s   - Specifies optional segment number.\r\n"
> +"  -s   - Specifies optional segment number (hexadecimal number).\r\n"
>  "  -i   - Displays interpreted information.\r\n"
> -"  Bus  - Specifies a bus number.\r\n"
> -"  Dev  - Specifies a device number.\r\n"
> -"  Func - Specifies a function number.\r\n"
> +"  -ec  - Displays detailed interpretation of specified PCIe extended
> capability\r\n"
> +"         ID (hexadecimal number).\r\n"
> +"  Bus  - Specifies a bus number (hexadecimal number).\r\n"
> +"  Dev  - Specifies a device number (hexadecimal number).\r\n"
> +"  Func - Specifies a function number (hexadecimal number).\r\n"
>  ".SH DESCRIPTION\r\n"
>  " \r\n"
>  "NOTES:\r\n"
> --
> 2.9.0.windows.1