* [PATCH v2 1/4] MdeModulePkg/PciHostBridge: Enhance boundary check in Io/Mem.Read/Write
2018-09-25 6:21 [PATCH v2 0/4] Fix a bug that prevents PMEM access Ruiyu Ni
@ 2018-09-25 6:21 ` Ruiyu Ni
2018-09-25 6:21 ` [PATCH v2 2/4] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access Ruiyu Ni
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Ruiyu Ni @ 2018-09-25 6:21 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
---
.../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 28 ++++++++++++++++++----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index f8a1239ceb..2c373e41de 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -301,6 +301,8 @@ CreateRootBridge (
@retval EFI_INVALID_PARAMETER Buffer is NULL.
+ @retval EFI_INVALID_PARAMETER Address or Count is invalid.
+
@retval EFI_UNSUPPORTED The Buffer is not aligned for the given Width.
@retval EFI_UNSUPPORTED The address range specified by Address, Width,
@@ -321,6 +323,7 @@ RootBridgeIoCheckParameter (
UINT64 Base;
UINT64 Limit;
UINT32 Size;
+ UINT64 Length;
//
// Check to see if Buffer is NULL
@@ -337,7 +340,7 @@ RootBridgeIoCheckParameter (
}
//
- // For FIFO type, the target address won't increase during the access,
+ // For FIFO type, the device address won't increase during the access,
// so treat Count as 1
//
if (Width >= EfiPciWidthFifoUint8 && Width <= EfiPciWidthFifoUint64) {
@@ -347,6 +350,13 @@ RootBridgeIoCheckParameter (
Width = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_WIDTH) (Width & 0x03);
Size = 1 << Width;
+ //
+ // Make sure (Count * Size) doesn't exceed MAX_UINT64
+ //
+ if (Count > DivU64x32 (MAX_UINT64, Size)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
//
// Check to see if Address is aligned
//
@@ -354,6 +364,14 @@ RootBridgeIoCheckParameter (
return EFI_UNSUPPORTED;
}
+ //
+ // Make sure (Address + Count * Size) doesn't exceed MAX_UINT64
+ //
+ Length = MultU64x32 (Count, Size);
+ if (Address > MAX_UINT64 - Length) {
+ return EFI_INVALID_PARAMETER;
+ }
+
RootBridge = ROOT_BRIDGE_FROM_THIS (This);
//
@@ -372,7 +390,7 @@ RootBridgeIoCheckParameter (
//
// Allow Legacy IO access
//
- if (Address + MultU64x32 (Count, Size) <= 0x1000) {
+ if (Address + Length <= 0x1000) {
if ((RootBridge->Attributes & (
EFI_PCI_ATTRIBUTE_ISA_IO | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO | EFI_PCI_ATTRIBUTE_VGA_IO |
EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO | EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO |
@@ -386,7 +404,7 @@ RootBridgeIoCheckParameter (
//
// Allow Legacy MMIO access
//
- if ((Address >= 0xA0000) && (Address + MultU64x32 (Count, Size)) <= 0xC0000) {
+ if ((Address >= 0xA0000) && (Address + Length) <= 0xC0000) {
if ((RootBridge->Attributes & EFI_PCI_ATTRIBUTE_VGA_MEMORY) != 0) {
return EFI_SUCCESS;
}
@@ -395,7 +413,7 @@ RootBridgeIoCheckParameter (
// By comparing the Address against Limit we know which range to be used
// for checking
//
- if (Address + MultU64x32 (Count, Size) <= RootBridge->Mem.Limit + 1) {
+ if (Address + Length <= RootBridge->Mem.Limit + 1) {
Base = RootBridge->Mem.Base;
Limit = RootBridge->Mem.Limit;
} else {
@@ -427,7 +445,7 @@ RootBridgeIoCheckParameter (
return EFI_INVALID_PARAMETER;
}
- if (Address + MultU64x32 (Count, Size) > Limit + 1) {
+ if (Address + Length > Limit + 1) {
return EFI_INVALID_PARAMETER;
}
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 2/4] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access
2018-09-25 6:21 [PATCH v2 0/4] Fix a bug that prevents PMEM access Ruiyu Ni
2018-09-25 6:21 ` [PATCH v2 1/4] MdeModulePkg/PciHostBridge: Enhance boundary check in Io/Mem.Read/Write Ruiyu Ni
@ 2018-09-25 6:21 ` Ruiyu Ni
2018-09-25 6:21 ` [PATCH v2 3/4] MdeModulePkg/PciHostBridge: Add RESOURCE_VALID() to simplify code Ruiyu Ni
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Ruiyu Ni @ 2018-09-25 6:21 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1196
RootBridgeIoCheckParameter() verifies that the requested MMIO access
can fit in any of the MEM/PMEM 32/64 ranges. But today's logic
somehow only checks the requested access against MEM 32/64 ranges.
It should also check the requested access against PMEM 32/64 ranges.
The patch fixes this issue.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
---
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 2c373e41de..87385aa172 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -413,12 +413,18 @@ RootBridgeIoCheckParameter (
// By comparing the Address against Limit we know which range to be used
// for checking
//
- if (Address + Length <= RootBridge->Mem.Limit + 1) {
- Base = RootBridge->Mem.Base;
+ if ((Address >= RootBridge->Mem.Base) && (Address + Length <= RootBridge->Mem.Limit + 1)) {
+ Base = RootBridge->Mem.Base;
Limit = RootBridge->Mem.Limit;
- } else {
- Base = RootBridge->MemAbove4G.Base;
+ } else if ((Address >= RootBridge->PMem.Base) && (Address + Length <= RootBridge->PMem.Limit + 1)) {
+ Base = RootBridge->PMem.Base;
+ Limit = RootBridge->PMem.Limit;
+ } else if ((Address >= RootBridge->MemAbove4G.Base) && (Address + Length <= RootBridge->MemAbove4G.Limit + 1)) {
+ Base = RootBridge->MemAbove4G.Base;
Limit = RootBridge->MemAbove4G.Limit;
+ } else {
+ Base = RootBridge->PMemAbove4G.Base;
+ Limit = RootBridge->PMemAbove4G.Limit;
}
} else {
PciRbAddr = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS*) &Address;
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 3/4] MdeModulePkg/PciHostBridge: Add RESOURCE_VALID() to simplify code
2018-09-25 6:21 [PATCH v2 0/4] Fix a bug that prevents PMEM access Ruiyu Ni
2018-09-25 6:21 ` [PATCH v2 1/4] MdeModulePkg/PciHostBridge: Enhance boundary check in Io/Mem.Read/Write Ruiyu Ni
2018-09-25 6:21 ` [PATCH v2 2/4] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access Ruiyu Ni
@ 2018-09-25 6:21 ` Ruiyu Ni
2018-09-25 6:21 ` [PATCH v2 4/4] MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file Ruiyu Ni
2018-09-25 12:12 ` [PATCH v2 0/4] Fix a bug that prevents PMEM access Zeng, Star
4 siblings, 0 replies; 8+ messages in thread
From: Ruiyu Ni @ 2018-09-25 6:21 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
---
.../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 26 ++++++++++------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 87385aa172..16413b60a6 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -21,6 +21,8 @@ extern EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
#define NO_MAPPING (VOID *) (UINTN) -1
+#define RESOURCE_VALID(Resource) ((Resource)->Base <= (Resource)->Limit)
+
//
// Lookup table for increment values based on transfer widths
//
@@ -122,25 +124,25 @@ CreateRootBridge (
//
// Make sure Mem and MemAbove4G apertures are valid
//
- if (Bridge->Mem.Base <= Bridge->Mem.Limit) {
+ if (RESOURCE_VALID (&Bridge->Mem)) {
ASSERT (Bridge->Mem.Limit < SIZE_4GB);
if (Bridge->Mem.Limit >= SIZE_4GB) {
return NULL;
}
}
- if (Bridge->MemAbove4G.Base <= Bridge->MemAbove4G.Limit) {
+ if (RESOURCE_VALID (&Bridge->MemAbove4G)) {
ASSERT (Bridge->MemAbove4G.Base >= SIZE_4GB);
if (Bridge->MemAbove4G.Base < SIZE_4GB) {
return NULL;
}
}
- if (Bridge->PMem.Base <= Bridge->PMem.Limit) {
+ if (RESOURCE_VALID (&Bridge->PMem)) {
ASSERT (Bridge->PMem.Limit < SIZE_4GB);
if (Bridge->PMem.Limit >= SIZE_4GB) {
return NULL;
}
}
- if (Bridge->PMemAbove4G.Base <= Bridge->PMemAbove4G.Limit) {
+ if (RESOURCE_VALID (&Bridge->PMemAbove4G)) {
ASSERT (Bridge->PMemAbove4G.Base >= SIZE_4GB);
if (Bridge->PMemAbove4G.Base < SIZE_4GB) {
return NULL;
@@ -157,11 +159,9 @@ CreateRootBridge (
// support separate windows for Non-prefetchable and Prefetchable
// memory.
//
- ASSERT (Bridge->PMem.Base > Bridge->PMem.Limit);
- ASSERT (Bridge->PMemAbove4G.Base > Bridge->PMemAbove4G.Limit);
- if ((Bridge->PMem.Base <= Bridge->PMem.Limit) ||
- (Bridge->PMemAbove4G.Base <= Bridge->PMemAbove4G.Limit)
- ) {
+ ASSERT (!RESOURCE_VALID (&Bridge->PMem));
+ ASSERT (!RESOURCE_VALID (&Bridge->PMemAbove4G));
+ if (RESOURCE_VALID (&Bridge->PMem) || RESOURCE_VALID (&Bridge->PMemAbove4G)) {
return NULL;
}
}
@@ -171,11 +171,9 @@ CreateRootBridge (
// If this bit is not set, then the PCI Root Bridge does not support
// 64 bit memory windows.
//
- ASSERT (Bridge->MemAbove4G.Base > Bridge->MemAbove4G.Limit);
- ASSERT (Bridge->PMemAbove4G.Base > Bridge->PMemAbove4G.Limit);
- if ((Bridge->MemAbove4G.Base <= Bridge->MemAbove4G.Limit) ||
- (Bridge->PMemAbove4G.Base <= Bridge->PMemAbove4G.Limit)
- ) {
+ ASSERT (!RESOURCE_VALID (&Bridge->MemAbove4G));
+ ASSERT (!RESOURCE_VALID (&Bridge->PMemAbove4G));
+ if (RESOURCE_VALID (&Bridge->MemAbove4G) || RESOURCE_VALID (&Bridge->PMemAbove4G)) {
return NULL;
}
}
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2 4/4] MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file
2018-09-25 6:21 [PATCH v2 0/4] Fix a bug that prevents PMEM access Ruiyu Ni
` (2 preceding siblings ...)
2018-09-25 6:21 ` [PATCH v2 3/4] MdeModulePkg/PciHostBridge: Add RESOURCE_VALID() to simplify code Ruiyu Ni
@ 2018-09-25 6:21 ` Ruiyu Ni
2018-09-25 10:43 ` Laszlo Ersek
2018-09-25 10:44 ` Laszlo Ersek
2018-09-25 12:12 ` [PATCH v2 0/4] Fix a bug that prevents PMEM access Zeng, Star
4 siblings, 2 replies; 8+ messages in thread
From: Ruiyu Ni @ 2018-09-25 6:21 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng
The change doesn't have functionality impact.
It just renames the mIoMmuProtocol to mIoMmu and moves the\
declaration from PciRootBridgeIo.c to PciHostBridge.h.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Suggested-by: Star Zeng <star.zeng@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 4 +-
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.h | 3 ++
.../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 1 -
.../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 60 +++++++++++-----------
4 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 0c1f75efcb..a74c6f0d30 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -26,7 +26,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mPciResourceTypeStr[] = {
L"I/O", L"Mem", L"PMem", L"Mem64", L"PMem64", L"Bus"
};
-EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
+EDKII_IOMMU_PROTOCOL *mIoMmu;
EFI_EVENT mIoMmuEvent;
VOID *mIoMmuRegistration;
@@ -363,7 +363,7 @@ IoMmuProtocolCallback (
{
EFI_STATUS Status;
- Status = gBS->LocateProtocol (&gEdkiiIoMmuProtocolGuid, NULL, (VOID **)&mIoMmuProtocol);
+ Status = gBS->LocateProtocol (&gEdkiiIoMmuProtocolGuid, NULL, (VOID **)&mIoMmu);
if (!EFI_ERROR(Status)) {
gBS->CloseEvent (mIoMmuEvent);
}
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
index bc9c7214dd..e0ed39eebc 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
@@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Library/MemoryAllocationLib.h>
#include <Library/PciHostBridgeLib.h>
#include <Protocol/PciHostBridgeResourceAllocation.h>
+#include <Protocol/IoMmu.h>
#include "PciRootBridge.h"
@@ -269,4 +270,6 @@ GetTranslationByResourceType (
);
extern EFI_CPU_IO2_PROTOCOL *mCpuIo;
+extern EDKII_IOMMU_PROTOCOL *mIoMmu;
+
#endif
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
index 06871052e7..e8513c906f 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
@@ -26,7 +26,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/CpuIo2.h>
#include <Protocol/DevicePath.h>
#include <Protocol/PciRootBridgeIo.h>
-#include <Protocol/IoMmu.h>
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
#include <Library/BaseMemoryLib.h>
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 16413b60a6..4c908fad88 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -17,8 +17,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "PciRootBridge.h"
#include "PciHostResource.h"
-extern EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
-
#define NO_MAPPING (VOID *) (UINTN) -1
#define RESOURCE_VALID(Resource) ((Resource)->Base <= (Resource)->Limit)
@@ -1269,7 +1267,7 @@ RootBridgeIoMap (
RootBridge = ROOT_BRIDGE_FROM_THIS (This);
- if (mIoMmuProtocol != NULL) {
+ if (mIoMmu != NULL) {
if (!RootBridge->DmaAbove4G) {
//
// Clear 64bit support
@@ -1278,14 +1276,14 @@ RootBridgeIoMap (
Operation = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) (Operation - EfiPciOperationBusMasterRead64);
}
}
- Status = mIoMmuProtocol->Map (
- mIoMmuProtocol,
- (EDKII_IOMMU_OPERATION) Operation,
- HostAddress,
- NumberOfBytes,
- DeviceAddress,
- Mapping
- );
+ Status = mIoMmu->Map (
+ mIoMmu,
+ (EDKII_IOMMU_OPERATION) Operation,
+ HostAddress,
+ NumberOfBytes,
+ DeviceAddress,
+ Mapping
+ );
return Status;
}
@@ -1413,11 +1411,11 @@ RootBridgeIoUnmap (
PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
EFI_STATUS Status;
- if (mIoMmuProtocol != NULL) {
- Status = mIoMmuProtocol->Unmap (
- mIoMmuProtocol,
- Mapping
- );
+ if (mIoMmu != NULL) {
+ Status = mIoMmu->Unmap (
+ mIoMmu,
+ Mapping
+ );
return Status;
}
@@ -1539,21 +1537,21 @@ RootBridgeIoAllocateBuffer (
RootBridge = ROOT_BRIDGE_FROM_THIS (This);
- if (mIoMmuProtocol != NULL) {
+ if (mIoMmu != NULL) {
if (!RootBridge->DmaAbove4G) {
//
// Clear DUAL_ADDRESS_CYCLE
//
Attributes &= ~((UINT64) EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE);
}
- Status = mIoMmuProtocol->AllocateBuffer (
- mIoMmuProtocol,
- Type,
- MemoryType,
- Pages,
- HostAddress,
- Attributes
- );
+ Status = mIoMmu->AllocateBuffer (
+ mIoMmu,
+ Type,
+ MemoryType,
+ Pages,
+ HostAddress,
+ Attributes
+ );
return Status;
}
@@ -1603,12 +1601,12 @@ RootBridgeIoFreeBuffer (
{
EFI_STATUS Status;
- if (mIoMmuProtocol != NULL) {
- Status = mIoMmuProtocol->FreeBuffer (
- mIoMmuProtocol,
- Pages,
- HostAddress
- );
+ if (mIoMmu != NULL) {
+ Status = mIoMmu->FreeBuffer (
+ mIoMmu,
+ Pages,
+ HostAddress
+ );
return Status;
}
--
2.16.1.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 4/4] MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file
2018-09-25 6:21 ` [PATCH v2 4/4] MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file Ruiyu Ni
@ 2018-09-25 10:43 ` Laszlo Ersek
2018-09-25 10:44 ` Laszlo Ersek
1 sibling, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2018-09-25 10:43 UTC (permalink / raw)
To: Ruiyu Ni, edk2-devel; +Cc: Star Zeng
On 09/25/18 08:21, Ruiyu Ni wrote:
> The change doesn't have functionality impact.
> It just renames the mIoMmuProtocol to mIoMmu and moves the\
> declaration from PciRootBridgeIo.c to PciHostBridge.h.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Suggested-by: Star Zeng <star.zeng@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> ---
> .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 4 +-
> .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.h | 3 ++
> .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 1 -
> .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 60 +++++++++++-----------
> 4 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> index 0c1f75efcb..a74c6f0d30 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> @@ -26,7 +26,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mPciResourceTypeStr[] = {
> L"I/O", L"Mem", L"PMem", L"Mem64", L"PMem64", L"Bus"
> };
>
> -EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
> +EDKII_IOMMU_PROTOCOL *mIoMmu;
> EFI_EVENT mIoMmuEvent;
> VOID *mIoMmuRegistration;
>
> @@ -363,7 +363,7 @@ IoMmuProtocolCallback (
> {
> EFI_STATUS Status;
>
> - Status = gBS->LocateProtocol (&gEdkiiIoMmuProtocolGuid, NULL, (VOID **)&mIoMmuProtocol);
> + Status = gBS->LocateProtocol (&gEdkiiIoMmuProtocolGuid, NULL, (VOID **)&mIoMmu);
> if (!EFI_ERROR(Status)) {
> gBS->CloseEvent (mIoMmuEvent);
> }
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
> index bc9c7214dd..e0ed39eebc 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
> @@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <Library/MemoryAllocationLib.h>
> #include <Library/PciHostBridgeLib.h>
> #include <Protocol/PciHostBridgeResourceAllocation.h>
> +#include <Protocol/IoMmu.h>
>
> #include "PciRootBridge.h"
>
> @@ -269,4 +270,6 @@ GetTranslationByResourceType (
> );
>
> extern EFI_CPU_IO2_PROTOCOL *mCpuIo;
> +extern EDKII_IOMMU_PROTOCOL *mIoMmu;
> +
> #endif
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> index 06871052e7..e8513c906f 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> @@ -26,7 +26,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <Protocol/CpuIo2.h>
> #include <Protocol/DevicePath.h>
> #include <Protocol/PciRootBridgeIo.h>
> -#include <Protocol/IoMmu.h>
> #include <Library/DebugLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/BaseMemoryLib.h>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> index 16413b60a6..4c908fad88 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -17,8 +17,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include "PciRootBridge.h"
> #include "PciHostResource.h"
>
> -extern EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
> -
> #define NO_MAPPING (VOID *) (UINTN) -1
>
> #define RESOURCE_VALID(Resource) ((Resource)->Base <= (Resource)->Limit)
> @@ -1269,7 +1267,7 @@ RootBridgeIoMap (
>
> RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>
> - if (mIoMmuProtocol != NULL) {
> + if (mIoMmu != NULL) {
> if (!RootBridge->DmaAbove4G) {
> //
> // Clear 64bit support
> @@ -1278,14 +1276,14 @@ RootBridgeIoMap (
> Operation = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) (Operation - EfiPciOperationBusMasterRead64);
> }
> }
> - Status = mIoMmuProtocol->Map (
> - mIoMmuProtocol,
> - (EDKII_IOMMU_OPERATION) Operation,
> - HostAddress,
> - NumberOfBytes,
> - DeviceAddress,
> - Mapping
> - );
> + Status = mIoMmu->Map (
> + mIoMmu,
> + (EDKII_IOMMU_OPERATION) Operation,
> + HostAddress,
> + NumberOfBytes,
> + DeviceAddress,
> + Mapping
> + );
> return Status;
> }
>
> @@ -1413,11 +1411,11 @@ RootBridgeIoUnmap (
> PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
> EFI_STATUS Status;
>
> - if (mIoMmuProtocol != NULL) {
> - Status = mIoMmuProtocol->Unmap (
> - mIoMmuProtocol,
> - Mapping
> - );
> + if (mIoMmu != NULL) {
> + Status = mIoMmu->Unmap (
> + mIoMmu,
> + Mapping
> + );
> return Status;
> }
>
> @@ -1539,21 +1537,21 @@ RootBridgeIoAllocateBuffer (
>
> RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>
> - if (mIoMmuProtocol != NULL) {
> + if (mIoMmu != NULL) {
> if (!RootBridge->DmaAbove4G) {
> //
> // Clear DUAL_ADDRESS_CYCLE
> //
> Attributes &= ~((UINT64) EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE);
> }
> - Status = mIoMmuProtocol->AllocateBuffer (
> - mIoMmuProtocol,
> - Type,
> - MemoryType,
> - Pages,
> - HostAddress,
> - Attributes
> - );
> + Status = mIoMmu->AllocateBuffer (
> + mIoMmu,
> + Type,
> + MemoryType,
> + Pages,
> + HostAddress,
> + Attributes
> + );
> return Status;
> }
>
> @@ -1603,12 +1601,12 @@ RootBridgeIoFreeBuffer (
> {
> EFI_STATUS Status;
>
> - if (mIoMmuProtocol != NULL) {
> - Status = mIoMmuProtocol->FreeBuffer (
> - mIoMmuProtocol,
> - Pages,
> - HostAddress
> - );
> + if (mIoMmu != NULL) {
> + Status = mIoMmu->FreeBuffer (
> + mIoMmu,
> + Pages,
> + HostAddress
> + );
> return Status;
> }
>
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2 4/4] MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file
2018-09-25 6:21 ` [PATCH v2 4/4] MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file Ruiyu Ni
2018-09-25 10:43 ` Laszlo Ersek
@ 2018-09-25 10:44 ` Laszlo Ersek
1 sibling, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2018-09-25 10:44 UTC (permalink / raw)
To: Ruiyu Ni, edk2-devel; +Cc: Star Zeng
On 09/25/18 08:21, Ruiyu Ni wrote:
> The change doesn't have functionality impact.
> It just renames the mIoMmuProtocol to mIoMmu and moves the\
> declaration from PciRootBridgeIo.c to PciHostBridge.h.
Before pushing the patch, please remove the stray backslash from the
commit message.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/4] Fix a bug that prevents PMEM access
2018-09-25 6:21 [PATCH v2 0/4] Fix a bug that prevents PMEM access Ruiyu Ni
` (3 preceding siblings ...)
2018-09-25 6:21 ` [PATCH v2 4/4] MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file Ruiyu Ni
@ 2018-09-25 12:12 ` Zeng, Star
4 siblings, 0 replies; 8+ messages in thread
From: Zeng, Star @ 2018-09-25 12:12 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com> with Laszlo's minor comment handled.
Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, September 25, 2018 2:21 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2 0/4] Fix a bug that prevents PMEM access
The patch sets fix a bug in PciHostBridge driver that prevents PMEM access sometimes. Additionally, it enhances the boundary check and add a new macro to simplify the code.
V2: Patch 3/4 uses Resource as parameter instead of R and treat the parameter
as a pointer.
Patch 4/4 is a new patch suggested by Star to move declaration of mIoMmu to
header file.
Ruiyu Ni (4):
MdeModulePkg/PciHostBridge: Enhance boundary check in
Io/Mem.Read/Write
MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access
MdeModulePkg/PciHostBridge: Add RESOURCE_VALID() to simplify code
MdeModulePkg/PciHostBridge: Move declaration of mIoMmu to header file
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 4 +-
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.h | 3 +
.../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 1 -
.../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 126 ++++++++++++---------
4 files changed, 78 insertions(+), 56 deletions(-)
--
2.16.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread