Re: [edk2-devel] [PATCH v5 06/12] OvmfPkg/MptScsiDxe: Report targets and one LUN

["Liran Alon" <liran.alon@oracle.com>]

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 <lersek@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.

-Liran