From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::243; helo=mail-io0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) (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 F1DEE207E5412 for ; Thu, 24 May 2018 01:15:50 -0700 (PDT) Received: by mail-io0-x243.google.com with SMTP id o185-v6so1273970iod.0 for ; Thu, 24 May 2018 01:15:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=xpSQqvVKo1g7KL1qOv6oKulxSjiG2jGVeF1e1y9MZ+0=; b=YGRcKzbZ0OBnqnYyIB8dxcOlUsg2gFEG8LZT0Khe4CiHW4/IooVwuioXV+IV9rMt5W 2GrtwiB5gzXArR4TZgwGrIRrNinHohJNGrNT5JA7ey7vfEjKrqDg8yS3ItgrAHowIaYf RiJUtjxEAZKChCW6V8GUpEj3AzvHE9+4tGIw0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=xpSQqvVKo1g7KL1qOv6oKulxSjiG2jGVeF1e1y9MZ+0=; b=FDS8miCgWAZtUDv7Ljrx/0yZRv+E1dVgGRIE6QR2rZ54cyIR4jaksEvGizlGLcF0mF Zsif/NxHl1BnWYT8Bz+H889TxW1skaAtf2B6Mjy+5BTv7ovO3iv9aOBePvN387YMwfjK SDZxs9iKz3yQhmfEN/BDRUaTwK0Q2lBUcKHFK1iEmb9oQu0NoU5NgmWvAnfJc/u7v8se 0L42gYIndwS0a+geV5G5PwG8YT2eoCzTZvFMasPOYf/1KLoYTZCDCAkg+0fJf6652i7b rOdUB9Fq29MUDEWOehl5yJ1H3jMpz7hbJ38LicdKQUM1D7rudu4pRkQAfTVPi7hX3odQ RF8w== X-Gm-Message-State: ALKqPwfJDIowmnBSWxnTJayTPmOSGV+03+m4aUKLu4vatUEH90wDXsGl uQSFSFVG27cAgRD8n+RXbiGwBAJFbriWegVYdn+mFEijYDI= X-Google-Smtp-Source: AB8JxZoYWsk1QayWkDDMlGp4OuZw2cdX8tcePaWpV2/r90rov1u7KH9IjFPlxHhlQbHtk06xNEXNt7cwD/eEQM9DQwY= X-Received: by 2002:a6b:545:: with SMTP id 66-v6mr5670214iof.173.1527149750112; Thu, 24 May 2018 01:15:50 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Thu, 24 May 2018 01:15:49 -0700 (PDT) In-Reply-To: <20180523202121.8125-7-lersek@redhat.com> References: <20180523202121.8125-1-lersek@redhat.com> <20180523202121.8125-7-lersek@redhat.com> From: Ard Biesheuvel Date: Thu, 24 May 2018 10:15:49 +0200 Message-ID: To: Laszlo Ersek Cc: edk2-devel-01 , Jordan Justen Subject: Re: [PATCH v2 6/7] OvmfPkg/PciHotPlugInitDxe: convert to PciCapLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 May 2018 08:15:51 -0000 Content-Type: text/plain; charset="UTF-8" On 23 May 2018 at 22:21, Laszlo Ersek wrote: > Replace the manual capability list parsing in OvmfPkg/PciHotPlugInitDxe > with PciCapLib and PciCapPciSegmentLib API calls. > > Cc: Ard Biesheuvel > Cc: Jordan Justen > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek Reviewed-by: Ard Biesheuvel > --- > > Notes: > v2: > - no changes > > OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 5 + > OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 267 +++++++------------- > 2 files changed, 102 insertions(+), 170 deletions(-) > > diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf > index 38043986eb67..cc2b60d44263 100644 > --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf > +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf > @@ -35,6 +35,8 @@ [LibraryClasses] > DebugLib > DevicePathLib > MemoryAllocationLib > + PciCapLib > + PciCapPciSegmentLib > PciLib > UefiBootServicesTableLib > UefiDriverEntryPoint > @@ -42,5 +44,8 @@ [LibraryClasses] > [Protocols] > gEfiPciHotPlugInitProtocolGuid ## ALWAYS_PRODUCES > > +[Pcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId ## CONSUMES > + > [Depex] > TRUE > diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c > index 177e1a62120d..3449796878ef 100644 > --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c > +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c > @@ -14,6 +14,7 @@ > **/ > > #include > +#include > #include > > #include > @@ -21,12 +22,20 @@ > #include > #include > #include > +#include > +#include > #include > #include > > #include > #include > > +// > +// TRUE if the PCI platform supports extended config space, FALSE otherwise. > +// > +STATIC BOOLEAN mPciExtConfSpaceSupported; > + > + > // > // The protocol interface this driver produces. > // > @@ -248,91 +257,11 @@ HighBitSetRoundUp64 ( > } > > > -/** > - Read a slice from conventional PCI config space at the given offset, then > - advance the offset. > - > - @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function > - -- in UEFI (not PciLib) encoding. > - > - @param[in,out] Offset On input, the offset in conventional PCI config space > - to start reading from. On output, the offset of the > - first byte that was not read. > - > - @param[in] Size The number of bytes to read. > - > - @param[out] Buffer On output, the bytes read from PCI config space are > - stored in this object. > -**/ > -STATIC > -VOID > -ReadConfigSpace ( > - IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *PciAddress, > - IN OUT UINT8 *Offset, > - IN UINT8 Size, > - OUT VOID *Buffer > - ) > -{ > - PciReadBuffer ( > - PCI_LIB_ADDRESS ( > - PciAddress->Bus, > - PciAddress->Device, > - PciAddress->Function, > - *Offset > - ), > - Size, > - Buffer > - ); > - *Offset += Size; > -} > - > - > -/** > - Convenience wrapper macro for ReadConfigSpace(). > - > - Given the following conditions: > - > - - HeaderField is the first field in the structure pointed-to by Struct, > - > - - Struct->HeaderField has been populated from the conventional PCI config > - space of the PCI device identified by PciAddress, > - > - - *Offset points one past HeaderField in the conventional PCI config space of > - the PCI device identified by PciAddress, > - > - populate the rest of *Struct from conventional PCI config space, starting at > - *Offset. Finally, increment *Offset so that it point one past *Struct. > - > - @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function > - -- in UEFI (not PciLib) encoding. Type: pointer to > - CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS. > - > - @param[in,out] Offset On input, the offset in conventional PCI config space > - to start reading from; one past Struct->HeaderField. > - On output, the offset of the first byte that was not > - read; one past *Struct. Type: pointer to UINT8. > - > - @param[out] Struct The structure to complete. Type: pointer to structure > - object. > - > - @param[in] HeaderField The name of the first field in *Struct, after which > - *Struct should be populated. Type: structure member > - identifier. > -**/ > -#define COMPLETE_CONFIG_SPACE_STRUCT(PciAddress, Offset, Struct, HeaderField) \ > - ReadConfigSpace ( \ > - (PciAddress), \ > - (Offset), \ > - (UINT8)(sizeof *(Struct) - sizeof ((Struct)->HeaderField)), \ > - &((Struct)->HeaderField) + 1 \ > - ) > - > - > /** > Look up the QEMU-specific Resource Reservation capability in the conventional > config space of a Hotplug Controller (that is, PCI Bridge). > > - This function performs as few config space reads as possible. > + On error, the contents of ReservationHint are indeterminate. > > @param[in] HpcPciAddress The address of the PCI Bridge -- Bus, Device, > Function -- in UEFI (not PciLib) encoding. > @@ -343,8 +272,9 @@ ReadConfigSpace ( > @retval EFI_SUCCESS The capability has been found, ReservationHint has > been populated. > > - @retval EFI_NOT_FOUND The capability is missing. The contents of > - ReservationHint are now indeterminate. > + @retval EFI_NOT_FOUND The capability is missing. > + > + @return Error codes from PciCapPciSegmentLib and PciCapLib. > **/ > STATIC > EFI_STATUS > @@ -353,10 +283,12 @@ QueryReservationHint ( > OUT QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION *ReservationHint > ) > { > - UINT16 PciVendorId; > - UINT16 PciStatus; > - UINT8 PciCapPtr; > - UINT8 Offset; > + UINT16 PciVendorId; > + EFI_STATUS Status; > + PCI_CAP_DEV *PciDevice; > + PCI_CAP_LIST *CapList; > + UINT16 VendorInstance; > + PCI_CAP *VendorCap; > > // > // Check the vendor identifier. > @@ -374,108 +306,101 @@ QueryReservationHint ( > } > > // > - // Check the Capabilities List bit in the PCI Status Register. > + // Parse the capabilities lists. > // > - PciStatus = PciRead16 ( > - PCI_LIB_ADDRESS ( > - HpcPciAddress->Bus, > - HpcPciAddress->Device, > - HpcPciAddress->Function, > - PCI_PRIMARY_STATUS_OFFSET > - ) > - ); > - if ((PciStatus & EFI_PCI_STATUS_CAPABILITY) == 0) { > - return EFI_NOT_FOUND; > + Status = PciCapPciSegmentDeviceInit ( > + mPciExtConfSpaceSupported ? PciCapExtended : PciCapNormal, > + 0, // Segment > + HpcPciAddress->Bus, > + HpcPciAddress->Device, > + HpcPciAddress->Function, > + &PciDevice > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + Status = PciCapListInit (PciDevice, &CapList); > + if (EFI_ERROR (Status)) { > + goto UninitPciDevice; > } > > // > - // Fetch the start of the Capabilities List. > - // > - PciCapPtr = PciRead8 ( > - PCI_LIB_ADDRESS ( > - HpcPciAddress->Bus, > - HpcPciAddress->Device, > - HpcPciAddress->Function, > - PCI_CAPBILITY_POINTER_OFFSET > - ) > - ); > - > - // > - // Scan the Capabilities List until we find the terminator element, or the > - // Resource Reservation capability. > + // Scan the vendor capability instances for the Resource Reservation > + // capability. > // > - for (Offset = PciCapPtr & 0xFC; > - Offset > 0; > - Offset = ReservationHint->BridgeHdr.VendorHdr.Hdr.NextItemPtr & 0xFC) { > - BOOLEAN EnoughRoom; > - > - // > - // Check if the Resource Reservation capability would fit into config space > - // at this offset. > - // > - EnoughRoom = (BOOLEAN)( > - Offset <= PCI_MAX_CONFIG_OFFSET - sizeof *ReservationHint > - ); > + VendorInstance = 0; > + for (;;) { > + UINT8 VendorLength; > + UINT8 BridgeCapType; > > - // > - // Read the standard capability header so we can check the capability ID > - // (if necessary) and advance to the next capability. > - // > - ReadConfigSpace ( > - HpcPciAddress, > - &Offset, > - (UINT8)sizeof ReservationHint->BridgeHdr.VendorHdr.Hdr, > - &ReservationHint->BridgeHdr.VendorHdr.Hdr > - ); > - if (!EnoughRoom || > - (ReservationHint->BridgeHdr.VendorHdr.Hdr.CapabilityID != > - EFI_PCI_CAPABILITY_ID_VENDOR)) { > - continue; > + Status = PciCapListFindCap ( > + CapList, > + PciCapNormal, > + EFI_PCI_CAPABILITY_ID_VENDOR, > + VendorInstance++, > + &VendorCap > + ); > + if (EFI_ERROR (Status)) { > + goto UninitCapList; > } > > // > - // Read the rest of the vendor capability header so we can check the > - // capability length. > + // Check the vendor capability length. > // > - COMPLETE_CONFIG_SPACE_STRUCT ( > - HpcPciAddress, > - &Offset, > - &ReservationHint->BridgeHdr.VendorHdr, > - Hdr > - ); > - if (ReservationHint->BridgeHdr.VendorHdr.Length != > - sizeof *ReservationHint) { > + Status = PciCapRead ( > + PciDevice, > + VendorCap, > + OFFSET_OF (EFI_PCI_CAPABILITY_VENDOR_HDR, Length), > + &VendorLength, > + sizeof VendorLength > + ); > + if (EFI_ERROR (Status)) { > + goto UninitCapList; > + } > + if (VendorLength != sizeof *ReservationHint) { > continue; > } > > // > - // Read the rest of the QEMU bridge capability header so we can check the > - // capability type. > + // Check the vendor bridge capability type. > // > - COMPLETE_CONFIG_SPACE_STRUCT ( > - HpcPciAddress, > - &Offset, > - &ReservationHint->BridgeHdr, > - VendorHdr > - ); > - if (ReservationHint->BridgeHdr.Type != > + Status = PciCapRead ( > + PciDevice, > + VendorCap, > + OFFSET_OF (QEMU_PCI_BRIDGE_CAPABILITY_HDR, Type), > + &BridgeCapType, > + sizeof BridgeCapType > + ); > + if (EFI_ERROR (Status)) { > + goto UninitCapList; > + } > + if (BridgeCapType == > QEMU_PCI_BRIDGE_CAPABILITY_TYPE_RESOURCE_RESERVATION) { > - continue; > + // > + // We have a match. > + // > + break; > } > - > - // > - // Read the body of the reservation hint. > - // > - COMPLETE_CONFIG_SPACE_STRUCT ( > - HpcPciAddress, > - &Offset, > - ReservationHint, > - BridgeHdr > - ); > - return EFI_SUCCESS; > } > > - return EFI_NOT_FOUND; > + // > + // Populate ReservationHint. > + // > + Status = PciCapRead ( > + PciDevice, > + VendorCap, > + 0, // SourceOffsetInCap > + ReservationHint, > + sizeof *ReservationHint > + ); > + > +UninitCapList: > + PciCapListUninit (CapList); > + > +UninitPciDevice: > + PciCapPciSegmentDeviceUninit (PciDevice); > + > + return Status; > } > > > @@ -870,6 +795,8 @@ DriverInitialize ( > { > EFI_STATUS Status; > > + mPciExtConfSpaceSupported = (PcdGet16 (PcdOvmfHostBridgePciDevId) == > + INTEL_Q35_MCH_DEVICE_ID); > mPciHotPlugInit.GetRootHpcList = GetRootHpcList; > mPciHotPlugInit.InitializeRootHpc = InitializeRootHpc; > mPciHotPlugInit.GetResourcePadding = GetResourcePadding; > -- > 2.14.1.3.gb7cf6e02401b > >