On 10 October 2018 at 10:55, Zeng, Star <[email protected]> wrote: > Reviewed-by: Star Zeng <[email protected]> >
Thanks. Pushed as 2730470f9d3b..1a3bee20820c > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Ard > Biesheuvel > Sent: Wednesday, October 10, 2018 4:35 PM > To: [email protected] > Cc: Zeng, Star <[email protected]> > Subject: [edk2] [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe: expose > unique B/D/F identifiers > > Currently, the implementation of EFI_PCI_IO_PROTOCOL::GetLocation() > in NonDiscoverablePciDeviceDxe returns the same set of dummy values > for each instance of the NON_DISCOVERABLE_DEVICE protocol that it > attaches itself to. However, this turns out to be causing problems > in cases where software (such as the ARM Compliance Test Suite [ACS]) > attempts to use these values to uniquely identify controllers, since > the collisions create ambiguity in this regard. > > So let's modify GetLocation() to return an arbitrary bus/device tuple > on segment 0xff instead. This is guaranteed not to clash with other > non-discoverable PCI devices, and highly unlikely to clash with real > PCIe devices. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <[email protected]> > Tested-by: Marcin Wojtas <[email protected]> > --- > v2: add sanity check on id value so we don't exceed 256 buses > initialize counter to 0 explicitly - this isn't strictly necessary but > clarifies > the code > > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c > | 10 ++++++++++ > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c > | 10 +++++++--- > > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h > | 6 ++++++ > 3 files changed, 23 insertions(+), 3 deletions(-) > > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c > index 3e9ff6620d8d..2483c7296912 100644 > --- > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c > @@ -16,6 +16,9 @@ > > #include <Protocol/DriverBinding.h> > > +#define MAX_NON_DISCOVERABLE_PCI_DEVICE_ID (32 * 256) > + > +STATIC UINTN mUniqueIdCounter = 0; > EFI_CPU_ARCH_PROTOCOL *mCpu; > > // > @@ -141,6 +144,11 @@ NonDiscoverablePciDeviceStart ( > NON_DISCOVERABLE_PCI_DEVICE *Dev; > EFI_STATUS Status; > > + ASSERT (mUniqueIdCounter < MAX_NON_DISCOVERABLE_PCI_DEVICE_ID); > + if (mUniqueIdCounter >= MAX_NON_DISCOVERABLE_PCI_DEVICE_ID) { > + return EFI_OUT_OF_RESOURCES; > + } > + > Dev = AllocateZeroPool (sizeof *Dev); > if (Dev == NULL) { > return EFI_OUT_OF_RESOURCES; > @@ -167,6 +175,8 @@ NonDiscoverablePciDeviceStart ( > goto CloseProtocol; > } > > + Dev->UniqueId = mUniqueIdCounter++; > + > return EFI_SUCCESS; > > CloseProtocol: > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c > index 0e42ae4bf6ec..58cb5d8b1fc5 100644 > --- > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c > @@ -1181,6 +1181,8 @@ PciIoGetLocation ( > OUT UINTN *FunctionNumber > ) > { > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + > if (SegmentNumber == NULL || > BusNumber == NULL || > DeviceNumber == NULL || > @@ -1188,9 +1190,11 @@ PciIoGetLocation ( > return EFI_INVALID_PARAMETER; > } > > - *SegmentNumber = 0; > - *BusNumber = 0xff; > - *DeviceNumber = 0; > + Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + > + *SegmentNumber = 0xff; > + *BusNumber = Dev->UniqueId >> 5; > + *DeviceNumber = Dev->UniqueId & 0x1f; > *FunctionNumber = 0; > > return EFI_SUCCESS; > diff --git > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h > > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h > index e641189267ee..5b4c57fa2a8e 100644 > --- > a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h > +++ > b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.h > @@ -100,6 +100,12 @@ typedef struct { > // on behalf of this device > // > LIST_ENTRY UncachedAllocationList; > + // > + // Unique ID for this device instance: needed so that we can report unique > + // segment/bus/device number for each device instance. Note that this > number > + // may change when disconnecting/reconnecting the driver. > + // > + UINTN UniqueId; > } NON_DISCOVERABLE_PCI_DEVICE; > > /** > -- > 2.19.1 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
