[edk2-devel] [PATCH] MdeModulePkg/PciHostBridge: Update Mem and PMem Limit Checks

[Wasim Khan]

> -----Original Message-----
> From: Ni, Ray <ray.ni at intel.com>
> Sent: Thursday, April 23, 2020 5:07 PM
> To: Wasim Khan <wasim.khan at nxp.com>; devel at edk2.groups.io
> Cc: ard.biesheuvel at linaro.org; Varun Sethi <V.Sethi at nxp.com>; Wu, Hao A
> <hao.a.wu at intel.com>
> Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Update Mem and PMem
> Limit Checks
> 
> Thanks for fixing the check.
> 
> PCI_ROOT_BRIDGE_APERTURE.Base/Limit are device address while the memory
> space in GCD belongs to host domain.
> So, host address for Mem/Pmem should be below 4GB while device address can
> across 4GB.
> 


Hi Ray,
Thank you for the review. 
There are cases when we don't have PCIe host address below 4GB, and the PCIe HOST Address space is only available above 4GB. 
For Example, Mem: 40000000 - FFFFFFFF Translation=FFFFFF6000000000 will result in HOST Address = 0xA0FFFFFFFF . This is a valid use case, but below check will report ASSERT for this HOST ADDRESS. 


> Can you enhance the check as below?
>   ASSERT (TO_HOST_ADDRESS (Bridge->Mem.Limit, Bridge->Mem.Translation) <
> SIZE_4GB);
>   if (TO_HOST_ADDRESS (Bridge->Mem.Limit, Bridge->Mem.Translation) >=
> SIZE_4GB) {
>     return NULL;
>   }
> 
> It will look more precise and can detect invalid Mem/Pmem resource.
> 
> > -----Original Message-----
> > From: Wasim Khan <wasim.khan at nxp.com>
> > Sent: Thursday, April 23, 2020 6:44 PM
> > To: devel at edk2.groups.io
> > Cc: ard.biesheuvel at linaro.org; v.sethi at nxp.com; Wu, Hao A
> > <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>; Wasim Khan
> > <wasim.khan at nxp.com>
> > Subject: [PATCH] MdeModulePkg/PciHostBridge: Update Mem and PMem
> Limit
> > Checks
> >
> > With Address Translation Support, it is possible and also correct that
> > Mem and Pmem Limit cross the 4GB boundary.
> > Update the checks so that Mem/PMem Limit should not cross 4GB from the
> > Mem/PMem Base address.
> >
> > Signed-off-by: Wasim Khan <wasim.khan at nxp.com>
> > ---
> >  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > index d304fae..9cf7e98 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> > @@ -117,8 +117,8 @@ CreateRootBridge (
> >    // Make sure Mem and MemAbove4G apertures are valid
> >    //
> >    if (RESOURCE_VALID (&Bridge->Mem)) {
> > -    ASSERT (Bridge->Mem.Limit < SIZE_4GB);
> > -    if (Bridge->Mem.Limit >= SIZE_4GB) {
> > +    ASSERT (Bridge->Mem.Limit < (Bridge->Mem.Base + SIZE_4GB));
> > +    if (Bridge->Mem.Limit >= (Bridge->Mem.Base + SIZE_4GB)) {
> >        return NULL;
> >      }
> >    }
> > @@ -129,8 +129,8 @@ CreateRootBridge (
> >      }
> >    }
> >    if (RESOURCE_VALID (&Bridge->PMem)) {
> > -    ASSERT (Bridge->PMem.Limit < SIZE_4GB);
> > -    if (Bridge->PMem.Limit >= SIZE_4GB) {
> > +    ASSERT (Bridge->PMem.Limit < (Bridge->PMem.Base + SIZE_4GB));
> > +    if (Bridge->PMem.Limit >= (Bridge->PMem.Base + SIZE_4GB)) {
> >        return NULL;
> >      }
> >    }
> > --
> > 2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#57968): https://edk2.groups.io/g/devel/message/57968
Mute This Topic: https://groups.io/mt/73215737/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-