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 <[email protected]> > Cc: Star Zeng <[email protected]> > Cc: Kirkendall, Garrett <[email protected]> > --- > 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*) &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 <[email protected]> Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
