On 04/29/20 16:45, Liran Alon wrote: > > On 29/04/2020 16:39, Laszlo Ersek wrote: >> On 04/29/20 15:38, Laszlo Ersek wrote: >>> On 04/24/20 19:59, Nikita Leshenko wrote: >>>> diff --git a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf >>>> b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf >>>> index 9f7c98829ee1..4862ff9dd497 100644 >>>> --- a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf >>>> +++ b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf >>>> @@ -24,6 +24,7 @@ [Packages] >>>> OvmfPkg/OvmfPkg.dec >>>> [LibraryClasses] >>>> + BaseMemoryLib >>>> DebugLib >>>> MemoryAllocationLib >>>> UefiBootServicesTableLib >>>> @@ -33,3 +34,6 @@ [LibraryClasses] >>>> [Protocols] >>>> gEfiExtScsiPassThruProtocolGuid ## BY_START >>>> gEfiPciIoProtocolGuid ## TO_START >>>> + >>>> +[FixedPcd] >>>> + gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit ## CONSUMES >>>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >>>> index 28030391cff2..2d09444bbb16 100644 >>>> --- a/OvmfPkg/OvmfPkg.dec >>>> +++ b/OvmfPkg/OvmfPkg.dec >>>> @@ -163,6 +163,10 @@ [PcdsFixedAtBuild] >>>> # polling loop iteration. >>>> >>>> gUefiOvmfPkgTokenSpaceGuid.PcdPvScsiWaitForCmpStallInUsecs|5|UINT32|0x38 >>>> >>>> + ## Set the *inclusive* number of targets that MptScsi exposes >>>> for scan >>>> + # by ScsiBusDxe. >>>> + gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit|7|UINT8|0x39 >>>> + >>>> >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8 >>>> >>>> >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9 >>>> >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa >>>> >>> (1) <PcdLib.h> should be #include'd in this patch, and PcdLib should be >>> listed under [LibraryClasses]. >>> >>> On one hand, that's a minor wart. The driver (and the OVMF platform(s)) >>> build OK at this stage, in practice. (I tested that.) So this, per se, >>> does not justify a v6. >>> >>> On the other hand, even at the end of the series, the module does not >>> spell out the PcdLib dependency (neither #include nor [LibraryClasses]). >>> That's not so nice. >>> >>> But, I'll fix that up for you, if there's not going to be another reason >>> for a v6. >>> >>> With (1) addressed (by you, or by me): >>> >>> Reviewed-by: Laszlo Ersek <ler...@redhat.com> >>> >>> Thanks, >>> Laszlo >>> >> ... BTW I missed the same in the PvScsiDxe driver :/ >> >> Liran, can you post a patch for that, please? >> >> Thanks, >> Laszlo > > > Sure. Will do. Quite surprised it builds successfully without it. > BTW, VirtioScsi DXE driver also seems to be missing PcdLib dependency > both in #include and [LibraryClasses] as-well. > I can submit a patch for that as-well if you like.
Yes please! Such dependencies are easy to miss, on commonly used / low-level lib classes. Usually one of the higher-level lib class headers pulls in the lib class header in question. And the [LibraryClasses] part remains hidden because a lib instance already (explicitly or imlicitly) consumed makes the consumer module inherit the [LibraryClasses] dependency too. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#58322): https://edk2.groups.io/g/devel/message/58322 Mute This Topic: https://groups.io/mt/73247325/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-