Re: [edk2] [PATCH 2/3] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access

2018-09-24 Thread Zeng, Star 

On 2018/9/21 15:25, Ruiyu Ni wrote:

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 
Cc: Star Zeng 
Cc: Kirkendall, Garrett 


Reviewed-by: Star Zeng 

Thanks,
Star


---
  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 0b6b56f846..f6234b5d11 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -411,12 +411,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*) 



___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/3] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access

2018-09-24 Thread Ni, Ruiyu 

On 9/21/2018 7:06 PM, Laszlo Ersek wrote:

On 09/21/18 09:25, Ruiyu Ni wrote:

The interesting thing about this patch is that, if any one of the first
three branches is taken, then the final checks will automatically pass.
That's because, on the first three branches, we select the base & the
limit *because* the access falls between them. Therefore, in the end,
when we check whether the access falls between base and end, they
miraculously happen to do so. :)


The code was written like this to maximally share the final check code:)



Reviewed-by: Laszlo Ersek 

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel




--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/3] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access

2018-09-24 Thread Kirkendall, Garrett 



GARRETT KIRKENDALL
SMTS Firmware Engineer | CTE
7171 Southwest Parkway, Austin, TX 78735 USA 
AMD   facebook  |  amd.com

-Original Message-
From: Ruiyu Ni  
Sent: Friday, September 21, 2018 2:26 AM
To: edk2-devel@lists.01.org
Cc: Star Zeng ; Kirkendall; Kirkendall, Garrett 

Subject: [PATCH 2/3] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM 
access

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 
Cc: Star Zeng 
Cc: Kirkendall, Garrett 
---
 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 0b6b56f846..f6234b5d11 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -411,12 +411,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*) 
--
2.16.1.windows.1

Reviewed-by: Garrett Kirkendall 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/3] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access

2018-09-21 Thread Laszlo Ersek 
On 09/21/18 09:25, Ruiyu Ni wrote:
> 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 
> Cc: Star Zeng 
> Cc: Kirkendall, Garrett 
> ---
>  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 0b6b56f846..f6234b5d11 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -411,12 +411,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*) 
> 

The interesting thing about this patch is that, if any one of the first
three branches is taken, then the final checks will automatically pass.
That's because, on the first three branches, we select the base & the
limit *because* the access falls between them. Therefore, in the end,
when we check whether the access falls between base and end, they
miraculously happen to do so. :)

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/3] MdeModulePkg/PciHostBridge: Fix a bug that prevents PMEM access

2018-09-21 Thread Ruiyu Ni 
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 
Cc: Star Zeng 
Cc: Kirkendall, Garrett 
---
 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 0b6b56f846..f6234b5d11 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -411,12 +411,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*) 
-- 
2.16.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel