From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4864:20::142; helo=mail-it1-x142.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-it1-x142.google.com (mail-it1-x142.google.com [IPv6:2607:f8b0:4864:20::142]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 555CB2194D387 for ; Tue, 9 Oct 2018 08:07:40 -0700 (PDT) Received: by mail-it1-x142.google.com with SMTP id c23-v6so3097310itd.5 for ; Tue, 09 Oct 2018 08:07:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=T9Nh8EaQzIS171su3p4l7IrvK61m4hrTKHKQY5z3MXY=; b=cyTrvhRty5Ev0dzhJ6PfUoVvYEsKRnYSJLF1sBEZKmIGEEz0HYrZz///+g/QeNPhNy sEMhoMyq9IfSo6bCA9I5o8Y/cjt1Z27fIfJbOd/6wlhdUIuFWkM/Z8xg7MxzYxoBf+t4 Ycle73S4A3JvFufPQHN5PdM04bz4ZZUXp+nQUt59CIA1BbLOaWWL3IObZR2tQKDlqawa DmS75eWUT7bYPJEwAOmzhvLgHIKCbaf+nmRsKWEA1vA+7gE6q9+8bm8pXPaPL1K41NJ7 Wp8b2Nd4pPcMRyi6gNxRpDG5EgMcAqKq4HKhFFgGC2WI1r4tJZAVTwI13Ad0GEpWH1rw jTrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=T9Nh8EaQzIS171su3p4l7IrvK61m4hrTKHKQY5z3MXY=; b=ElTvPFHRNTAyvYuUNBvBkJeauuP5ahvoAsY+Zp3nSx26jmtEhGV7by5W+gcM8P+JNj 9jdsNCowMExhLl/0rq/gmhIGzAXrXFxFSUU2SZ3G6RQmefJAiZAqLmhqLhNx6Z5VVcYF NLDqEAkP48pKwzwAJfyaFvYrtZ7zczefbP7JP+uG/ltoflCGN3kB79xZzoI93ONNiliN 9oIrGN1Eson6vWvCpaKwKuCknfdcOp/BzOZV8+MPgAteb4TyyXCwjfISw5pCi5VnIeHn bhmIAA8VaRYo7aVw9tsyaNjKSLZZ9ndesGJUU+xAwDewfF1uWMGmL9Kw/TdQL9IryyGL WcyQ== X-Gm-Message-State: ABuFfoiNm0TCa5mP74TY+Xr+THjhFdziJWE5BCjnlvfjgLBSv1zdLlFh WK2lEtWwyq3JvrevgswC+/1qtLAQH3PPJlgdQlXIFRwo X-Google-Smtp-Source: ACcGV63vt8yyk3QjHYjcDtSBOxzTkm7sKtQ0KK744b+VhjNG0GYh0Fc2HHiI8Sc1cCd4ISG7NBgHLEgKM8Lr5EKZQk8= X-Received: by 2002:a02:5ccc:: with SMTP id w73-v6mr14585745jad.112.1539097659466; Tue, 09 Oct 2018 08:07:39 -0700 (PDT) MIME-Version: 1.0 References: <20181009140128.21378-1-ard.biesheuvel@linaro.org> In-Reply-To: <20181009140128.21378-1-ard.biesheuvel@linaro.org> From: Marcin Wojtas Date: Tue, 9 Oct 2018 17:07:27 +0200 Message-ID: To: Ard Biesheuvel Cc: edk2-devel-01 , Leif Lindholm , "Zeng, Star" , "Ni, Ruiyu" Subject: Re: [PATCH] MdeModulePkg/NonDiscoverablePciDeviceDxe: expose unique B/D/F identifiers X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Oct 2018 15:07:40 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ard, wt., 9 pa=C5=BA 2018 o 16:01 Ard Biesheuvel nap= isa=C5=82(a): > > 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 > --- > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDevic= eDxe.c | 4 ++++ > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDevic= eIo.c | 10 +++++++--- > MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDevic= eIo.h | 6 ++++++ > 3 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscover= ablePciDeviceDxe.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDi= scoverablePciDeviceDxe.c > index 3e9ff6620d8d..ed5256f3c7ea 100644 > --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci= DeviceDxe.c > +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci= DeviceDxe.c > @@ -18,6 +18,8 @@ > > EFI_CPU_ARCH_PROTOCOL *mCpu; > > +STATIC UINTN mUniqueIdCounter; > + > // > // We only support the following device types > // > @@ -167,6 +169,8 @@ NonDiscoverablePciDeviceStart ( > goto CloseProtocol; > } > > + Dev->UniqueId =3D mUniqueIdCounter++; > + > return EFI_SUCCESS; > > CloseProtocol: > diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscover= ablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDis= coverablePciDeviceIo.c > index 0e42ae4bf6ec..58cb5d8b1fc5 100644 > --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci= DeviceIo.c > +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci= DeviceIo.c > @@ -1181,6 +1181,8 @@ PciIoGetLocation ( > OUT UINTN *FunctionNumber > ) > { > + NON_DISCOVERABLE_PCI_DEVICE *Dev; > + > if (SegmentNumber =3D=3D NULL || > BusNumber =3D=3D NULL || > DeviceNumber =3D=3D NULL || > @@ -1188,9 +1190,11 @@ PciIoGetLocation ( > return EFI_INVALID_PARAMETER; > } > > - *SegmentNumber =3D 0; > - *BusNumber =3D 0xff; > - *DeviceNumber =3D 0; > + Dev =3D NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This); > + > + *SegmentNumber =3D 0xff; > + *BusNumber =3D Dev->UniqueId >> 5; > + *DeviceNumber =3D Dev->UniqueId & 0x1f; > *FunctionNumber =3D 0; > > return EFI_SUCCESS; > diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscover= ablePciDeviceIo.h b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDis= coverablePciDeviceIo.h > index e641189267ee..5b4c57fa2a8e 100644 > --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci= DeviceIo.h > +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci= DeviceIo.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 un= ique > + // 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; > Tested-by: Marcin Wojtas Thanks, Marcin