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

Ruiyu Ni posted 1 patch 7 years, 1 month ago
Failed in applying to current master (apply log)
.../HexEdit/HexEditorTypes.h                       |  9 ++-
.../UefiShellDebug1CommandsLib/HexEdit/MemImage.c  | 65 +---------------------
2 files changed, 6 insertions(+), 68 deletions(-)
[edk2] [PATCH] ShellPkg/hexeditor: Use CpuIo for memory access
Posted by Ruiyu Ni 7 years, 1 month ago
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg/hexeditor: Use CpuIo for memory access
Posted by Carsey, Jaben 7 years, 1 month ago
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg/hexeditor: Use CpuIo for memory access
Posted by Ard Biesheuvel 7 years, 1 month ago
On 1 November 2017 at 18:15, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> 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.
>>


This patch breaks the build on Clang.

ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MemImage.c>:142:35:
error: implicit conversion from enumeration type
'EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH' to different enumeration type
'EFI_CPU_IO_PROTOCOL_WIDTH' [-Werror,-Wenum-conversion]
                                  EfiPciWidthUint8,
                                  ^~~~~~~~~~~~~~~~
ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MemImage.c>:265:35:
error: implicit conversion from enumeration type
'EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH' to different enumeration type
'EFI_CPU_IO_PROTOCOL_WIDTH' [-Werror,-Wenum-conversion]
                                  EfiPciWidthUint8,
                                  ^~~~~~~~~~~~~~~~
2 errors generated.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel