Re: [edk2] [Patch V3] ShellPkg/mm: Fix mm to support multiple root bridge platform

2015-12-08 Thread Kinney, Michael D 
Ray,

In general the code changes looks really good.  The CPU I/O protocols is only 
conditionally used, so the INF should be SOMETIMES_CONSUMES for that protocol.

I can not apply the patch because of an issue with UNI file to try running it.  
Can you please try regenerating the patch against the latest version of trunk.

Thanks,

Mike


> -Original Message-
> From: Ni, Ruiyu
> Sent: Monday, December 7, 2015 9:15 PM
> To: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kin...@intel.com>
> Subject: RE: [edk2] [Patch V3] ShellPkg/mm: Fix mm to support multiple root 
> bridge platform
> 
> The V3 version moved the HasRootBridgeIo variable definition to the beginning 
> of function.
> And I sent it through GIT command in case the attachment in V2 mail is 
> blocked by the lists.01.org.
> 
> Regards,
> Ray
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu 
> Ni
> Sent: Tuesday, December 8, 2015 1:13 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Kinney, Michael D 
> <michael.d.kin...@intel.com>
> Subject: [edk2] [Patch V3] ShellPkg/mm: Fix mm to support multiple root 
> bridge platform
> 
> In multiple root bridge platforms, different root bridges may
> share the same segment but occupy different range of buses,
> or may occupy different segments.
> The fix is to find the correct root bridge IO instance by
> comparing not only the segment but also the bus ranges.
> It tries to access the MMIO and IO in the following order:
> PciRootBridgeIo, CpuIo and direct IO.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Michael.D.Kinney <michael.d.kin...@intel.com>
> ---
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c   | 917 
> ++---
>  .../UefiShellDebug1CommandsLib.h   |   3 +-
>  .../UefiShellDebug1CommandsLib.inf |   4 +-
>  .../UefiShellDebug1CommandsLib.uni | Bin 139696 -> 139230 
> bytes
>  ShellPkg/ShellPkg.dsc  |   1 +
>  5 files changed, 464 insertions(+), 461 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c 
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c
> index ca64f2c..2f22629 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c
> @@ -2,7 +2,7 @@
>Main file for Mm shell Debug1 function.
> 
>(C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> -  Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.
> +  Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
>which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -15,16 +15,25 @@
> 
>  #include "UefiShellDebug1CommandsLib.h"
>  #include 
> +#include 
>  #include 
>  #include 
> 
>  typedef enum {
> -  EfiMemory,
> -  EFIMemoryMappedIo,
> -  EfiIo,
> -  EfiPciConfig,
> -  EfiPciEConfig
> -} EFI_ACCESS_TYPE;
> +  ShellMmMemory,
> +  ShellMmMemoryMappedIo,
> +  ShellMmIo,
> +  ShellMmPci,
> +  ShellMmPciExpress
> +} SHELL_MM_ACCESS_TYPE;
> +
> +CONST UINT16 mShellMmAccessTypeStr[] = {
> +  STRING_TOKEN (STR_MM_MEM),
> +  STRING_TOKEN (STR_MM_MMIO),
> +  STRING_TOKEN (STR_MM_IO),
> +  STRING_TOKEN (STR_MM_PCI),
> +  STRING_TOKEN (STR_MM_PCIE)
> +};
> 
>  STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>{L"-mmio", TypeFlag},
> @@ -37,161 +46,341 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>{NULL, TypeMax}
>};
> 
> -STATIC CONST UINT64 MaxNum[9]  = { 0xff, 0x, 0x, 
> 0xULL };
> +CONST UINT64 mShellMmMaxNumber[] = {
> +  0, MAX_UINT8, MAX_UINT16, 0, MAX_UINT32, 0, 0, 0, MAX_UINT64
> +};
> +CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH mShellMmRootBridgeIoWidth[] = {
> +  0, EfiPciWidthUint8, EfiPciWidthUint16, 0, EfiPciWidthUint32, 0, 0, 0, 
> EfiPciWidthUint64
> +};
> +CONST EFI_CPU_IO_PROTOCOL_WIDTH mShellMmCpuIoWidth[] = {
> +  0, EfiCpuIoWidthUint8, EfiCpuIoWidthUint16, 0, EfiCpuIoWidthUint32, 0, 0, 
> 0, EfiCpuIoWidthUint64
> +};
> 
>  /**
> -  Read some data into a buffer from memory.
> -
> -  @param[in] WidthThe width of each read.
> -  @param[in] Addresss The memory location to start reading at.
> -  @param[in] Size The size of Buffer in Width sized units.
> -  @param

Re: [edk2] [Patch V3] ShellPkg/mm: Fix mm to support multiple root bridge platform

2015-12-07 Thread Ni, Ruiyu 
The V3 version moved the HasRootBridgeIo variable definition to the beginning 
of function.
And I sent it through GIT command in case the attachment in V2 mail is blocked 
by the lists.01.org.

Regards,
Ray


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, December 8, 2015 1:13 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Kinney, Michael D 

Subject: [edk2] [Patch V3] ShellPkg/mm: Fix mm to support multiple root bridge 
platform

In multiple root bridge platforms, different root bridges may
share the same segment but occupy different range of buses,
or may occupy different segments.
The fix is to find the correct root bridge IO instance by
comparing not only the segment but also the bus ranges.
It tries to access the MMIO and IO in the following order:
PciRootBridgeIo, CpuIo and direct IO.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Michael.D.Kinney 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c   | 917 ++---
 .../UefiShellDebug1CommandsLib.h   |   3 +-
 .../UefiShellDebug1CommandsLib.inf |   4 +-
 .../UefiShellDebug1CommandsLib.uni | Bin 139696 -> 139230 bytes
 ShellPkg/ShellPkg.dsc  |   1 +
 5 files changed, 464 insertions(+), 461 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c
index ca64f2c..2f22629 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Mm.c
@@ -2,7 +2,7 @@
   Main file for Mm shell Debug1 function.
 
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -15,16 +15,25 @@
 
 #include "UefiShellDebug1CommandsLib.h"
 #include 
+#include 
 #include 
 #include 
 
 typedef enum {
-  EfiMemory,
-  EFIMemoryMappedIo,
-  EfiIo,
-  EfiPciConfig,
-  EfiPciEConfig
-} EFI_ACCESS_TYPE;
+  ShellMmMemory,
+  ShellMmMemoryMappedIo,
+  ShellMmIo,
+  ShellMmPci,
+  ShellMmPciExpress
+} SHELL_MM_ACCESS_TYPE;
+
+CONST UINT16 mShellMmAccessTypeStr[] = {
+  STRING_TOKEN (STR_MM_MEM),
+  STRING_TOKEN (STR_MM_MMIO),
+  STRING_TOKEN (STR_MM_IO),
+  STRING_TOKEN (STR_MM_PCI),
+  STRING_TOKEN (STR_MM_PCIE)
+};
 
 STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
   {L"-mmio", TypeFlag},
@@ -37,161 +46,341 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
   {NULL, TypeMax}
   };
 
-STATIC CONST UINT64 MaxNum[9]  = { 0xff, 0x, 0x, 
0xULL };
+CONST UINT64 mShellMmMaxNumber[] = {
+  0, MAX_UINT8, MAX_UINT16, 0, MAX_UINT32, 0, 0, 0, MAX_UINT64
+};
+CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH mShellMmRootBridgeIoWidth[] = {
+  0, EfiPciWidthUint8, EfiPciWidthUint16, 0, EfiPciWidthUint32, 0, 0, 0, 
EfiPciWidthUint64
+};
+CONST EFI_CPU_IO_PROTOCOL_WIDTH mShellMmCpuIoWidth[] = {
+  0, EfiCpuIoWidthUint8, EfiCpuIoWidthUint16, 0, EfiCpuIoWidthUint32, 0, 0, 0, 
EfiCpuIoWidthUint64
+};
 
 /**
-  Read some data into a buffer from memory.
-
-  @param[in] WidthThe width of each read.
-  @param[in] Addresss The memory location to start reading at.
-  @param[in] Size The size of Buffer in Width sized units.
-  @param[out] Buffer  The buffer to read into.
+  Extract the PCI segment, bus, device, function, register from
+  from a SHELL_MM_PCI or SHELL_MM_PCIE format of address..
+
+  @param[in]  PciFormat  Whether the address is of PCI format of PCIE 
format.
+  @param[in]  AddressSHELL_MM_PCI or SHELL_MM_PCIE address.
+  @param[out] SegmentPCI segment number.
+  @param[out] BusPCI bus number.
+  @param[out] Device PCI device number.
+  @param[out] Function   PCI function number.
+  @param[out] Register   PCI register offset.
 **/
 VOID
 EFIAPI
-ReadMem (
-  IN  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH  Width,
-  IN  UINT64Address,
-  IN  UINTN Size,
-  OUT VOID  *Buffer
+ShellMmDecodePciAddress (
+  IN BOOLEANPciFormat,
+  IN UINT64 Address,
+  OUT UINT32*Segment,
+  OUT UINT8 *Bus,
+  OUT UINT8 *Device,   OPTIONAL
+  OUT UINT8 *Function, OPTIONAL
+  OUT UINT32*Register  OPTIONAL
   )
 {
-  //
-  // This function is defective.  This ASSERT prevents the defect from 
affecting anything.
-  //
-  ASSERT(Size == 1);
-  do {
-if (Width == EfiPciWidthUint8) {
-  *(UINT8 *) Buffer = *(UINT8 *) (UINTN) Address;