From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.2022.1588183868645004549 for ; Wed, 29 Apr 2020 11:11:08 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Qlu4zJ7Y; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588183867; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8/wTs2oNnN1XYzDk+CMku6eRBc9X3giOmCSXBX3U+EU=; b=Qlu4zJ7Y0vFa/tO/6toV3rkpwc7QGHgZc5Oi71NplV3y/0Ypa6Dhmzi9n/EQZq7dpLMTJG X1NYqPZv270uL7gt3UjR6ButjyX4G4AqVQ4m+dB4+NKjsfcqso65kbnv7W8zALmIZcz0l4 hAW5/j10RSLf78WdW5ZXX4Q5v4p9DAg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-192-6rvoObkXMJGj-dvwiiBrOw-1; Wed, 29 Apr 2020 14:11:04 -0400 X-MC-Unique: 6rvoObkXMJGj-dvwiiBrOw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6E224A0BD7; Wed, 29 Apr 2020 18:11:02 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-1.ams2.redhat.com [10.36.114.1]) by smtp.corp.redhat.com (Postfix) with ESMTP id D534D61980; Wed, 29 Apr 2020 18:11:00 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v5 06/12] OvmfPkg/MptScsiDxe: Report targets and one LUN To: Liran Alon , devel@edk2.groups.io, nikita.leshchenko@oracle.com Cc: aaron.young@oracle.com, Jordan Justen , Ard Biesheuvel References: <20200424175927.41210-1-nikita.leshchenko@oracle.com> <20200424175927.41210-7-nikita.leshchenko@oracle.com> <4f6278f1-f04d-ce38-38f6-8d45401d0bf1@redhat.com> From: "Laszlo Ersek" Message-ID: <2666e264-2638-359b-ed59-60d19ecdcd61@redhat.com> Date: Wed, 29 Apr 2020 20:10:59 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable On 04/29/20 16:45, Liran Alon wrote: >=20 > 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] >>>> =A0=A0=A0 OvmfPkg/OvmfPkg.dec >>>> =A0 =A0 [LibraryClasses] >>>> +=A0 BaseMemoryLib >>>> =A0=A0=A0 DebugLib >>>> =A0=A0=A0 MemoryAllocationLib >>>> =A0=A0=A0 UefiBootServicesTableLib >>>> @@ -33,3 +34,6 @@ [LibraryClasses] >>>> =A0 [Protocols] >>>> =A0=A0=A0 gEfiExtScsiPassThruProtocolGuid=A0=A0=A0=A0=A0=A0=A0 ## BY_S= TART >>>> =A0=A0=A0 gEfiPciIoProtocolGuid=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0 ## TO_START >>>> + >>>> +[FixedPcd] >>>> +=A0 gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit=A0=A0 ## CONS= UMES >>>> 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] >>>> =A0=A0=A0 #=A0 polling loop iteration. >>>> =A0=A0=A0 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdPvScsiWaitForCmpStallInUsecs|5|UINT32|0x= 38 >>>> >>>> =A0 +=A0 ## Set the *inclusive* number of targets that MptScsi exposes >>>> for scan >>>> +=A0 #=A0 by ScsiBusDxe. >>>> +=A0 gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit|7|UINT8|0x39 >>>> + >>>> =A0=A0=A0 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT3= 2|0x8 >>>> >>>> =A0=A0=A0 >>>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT3= 2|0x9 >>>> >>>> =A0=A0=A0 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|= 0xa >>>> >>> (1) 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 reaso= n >>> for a v6. >>> >>> With (1) addressed (by you, or by me): >>> >>> Reviewed-by: Laszlo Ersek >>> >>> Thanks, >>> Laszlo >>> >> ... BTW I missed the same in the PvScsiDxe driver :/ >> >> Liran, can you post a patch for that, please? >> >> Thanks, >> Laszlo >=20 >=20 > 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