Re: [PATCH] ShellPkg/hexeditor: Use CpuIo for memory access

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

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

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Wednesday, November 01, 2017 3:13 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [PATCH] ShellPkg/hexeditor: Use CpuIo for memory access
> Importance: High
> 
> The original code uses PciRootBridgeIo for memory access.
> It worked before MdeModulePkg/PciHostBridgeDxe driver was checked in.
> But MdeModulePkg/PciHostBridgeDxe adds checks to ensure the MMIO
> access request is in the scope of the current RootBridgeIo instance.
> It causes "hexeditor -m 0 2" reports error because memory address 0
> surely is not in the scope of any RootBridgeIo instance.
> In fact only accessing the MMIO space occupied by the RootBridgeIo
> can work.
> 
> The patch changes hexeditor to use CpuIo for memory access.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> ---
>  .../HexEdit/HexEditorTypes.h                       |  9 ++-
>  .../UefiShellDebug1CommandsLib/HexEdit/MemImage.c  | 65 +----------------
> -----
>  2 files changed, 6 insertions(+), 68 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/HexEditorTypes.
> h
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/HexEditorTypes.
> h
> index ddd6070c94..8f0da3b667 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/HexEditorTypes.
> h
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/HexEditorTypes.
> h
> @@ -1,7 +1,7 @@
>  /** @file
>    data types that are used by editor
> 
> -  Copyright (c) 2005 - 2011, Intel Corporation. All rights reserved. <BR>
> +  Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved. <BR>
>    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
> @@ -78,10 +78,9 @@ typedef struct {
>  } HEFI_EDITOR_DISK_IMAGE;
> 
>  typedef struct {
> -  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL *IoFncs;
> -
> -  UINTN                           Offset;
> -  UINTN                           Size;
> +  EFI_CPU_IO2_PROTOCOL *IoFncs;
> +  UINTN                Offset;
> +  UINTN                Size;
>  } HEFI_EDITOR_MEM_IMAGE;
> 
>  typedef struct {
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MemImage.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MemImage.c
> index 300c67f0d4..fce9bbe0e6 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MemImage.c
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MemImage.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Functions to deal with Mem buffer
> 
> -  Copyright (c) 2005 - 2011, Intel Corporation. All rights reserved. <BR>
> +  Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved. <BR>
>    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
> @@ -27,8 +27,6 @@ extern HEFI_EDITOR_GLOBAL_EDITOR  HMainEditor;
>  HEFI_EDITOR_MEM_IMAGE             HMemImage;
>  HEFI_EDITOR_MEM_IMAGE             HMemImageBackupVar;
> 
> -EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   DummyPciRootBridgeIo;
> -
>  //
>  // for basic initialization of HDiskImage
>  //
> @@ -39,54 +37,6 @@ HEFI_EDITOR_MEM_IMAGE             HMemImageConst
> = {
>  };
> 
>  /**
> -  Empty function.  always returns the same.
> -
> -  @param[in] This        Ignored.
> -  @param[in] Width       Ignored.
> -  @param[in] Address     Ignored.
> -  @param[in] Count       Ignored.
> -  @param[in, out] Buffer Ignored.
> -
> -  @retval EFI_UNSUPPORTED.
> -**/
> -EFI_STATUS
> -EFIAPI
> -DummyMemRead (
> -  IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL              * This,
> -  IN     EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH    Width,
> -  IN     UINT64                                   Address,
> -  IN     UINTN                                    Count,
> -  IN OUT VOID                                     *Buffer
> -  )
> -{
> -  return EFI_UNSUPPORTED;
> -}
> -
> -/**
> -  Empty function.  always returns the same.
> -
> -  @param[in] This        Ignored.
> -  @param[in] Width       Ignored.
> -  @param[in] Address     Ignored.
> -  @param[in] Count       Ignored.
> -  @param[in, out] Buffer Ignored.
> -
> -  @retval EFI_UNSUPPORTED.
> -**/
> -EFI_STATUS
> -EFIAPI
> -DummyMemWrite (
> -  IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL              * This,
> -  IN     EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH    Width,
> -  IN     UINT64                                   Address,
> -  IN     UINTN                                    Count,
> -  IN OUT VOID                                     *Buffer
> -  )
> -{
> -  return EFI_UNSUPPORTED;
> -}
> -
> -/**
>    Initialization function for HDiskImage.
> 
>    @retval EFI_SUCCESS       The operation was successful.
> @@ -105,21 +55,10 @@ HMemImageInit (
>    CopyMem (&HMemImage, &HMemImageConst, sizeof (HMemImage));
> 
>    Status = gBS->LocateProtocol (
> -                &gEfiPciRootBridgeIoProtocolGuid,
> +                &gEfiCpuIo2ProtocolGuid,
>                  NULL,
>                  (VOID**)&HMemImage.IoFncs
>                  );
> -  if (Status == EFI_NOT_FOUND) {
> -    //
> -    // For NT32, no EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL is available
> -    // Use Dummy PciRootBridgeIo for memory access
> -    //
> -    ZeroMem (&DummyPciRootBridgeIo, sizeof
> (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL));
> -    DummyPciRootBridgeIo.Mem.Read  = DummyMemRead;
> -    DummyPciRootBridgeIo.Mem.Write = DummyMemWrite;
> -    HMemImage.IoFncs = &DummyPciRootBridgeIo;
> -    Status = EFI_SUCCESS;
> -  }
>    if (!EFI_ERROR (Status)) {
>      return EFI_SUCCESS;
>    } else {
> --
> 2.12.2.windows.2