From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web10.709.1595014834118722845 for ; Fri, 17 Jul 2020 12:40:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Kw6LgWW5; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1595014833; 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=DkAjlcvXrTm3/4V33tKSNklXkvCXzKSBpXy7kUSGK/U=; b=Kw6LgWW5GIVYkf27vBXghvUzsCxwpZPuzp4cqp+OiSCLcMhhR4U5ym3oE6qpX36gqlqtPd 8SMCO5Vnysopuyv+BDZAjGwsGBTvSMYvybP0A5ZyLDrklDYTm0nZtg/1H0HT01CjmBHxuP +P+BtnOh9SMU04a4ep4rr7vzyQxipDo= 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-215-mrbYqORoMM6rz1RIUhnqRA-1; Fri, 17 Jul 2020 15:40:31 -0400 X-MC-Unique: mrbYqORoMM6rz1RIUhnqRA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E91CD800464; Fri, 17 Jul 2020 19:40:29 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-68.ams2.redhat.com [10.36.112.68]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0E4887BD4E; Fri, 17 Jul 2020 19:40:26 +0000 (UTC) Subject: Re: [PATCH v3 06/11] OvmfPkg/LsiScsiDxe: Report Targets and LUNs To: Gary Lin , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel References: <20200717061130.8881-1-glin@suse.com> <20200717061130.8881-7-glin@suse.com> From: "Laszlo Ersek" Message-ID: Date: Fri, 17 Jul 2020 21:40:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200717061130.8881-7-glin@suse.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 07/17/20 08:11, Gary Lin wrote: > Implement LsiScsiGetNextTargetLun(), LsiScsiBuildDevicePath(), > LsiScsiGetTargetLun(), and LsiScsiGetNextTarget() to report Targets and > LUNs and build the device path. > > This commit also introduces two PCD value: PcdLsiScsiMaxTargetLimit and > PcdLsiScsiMaxLunLimit as the limits for Targets and LUNs. > > v3: > - Update the range of LUN in the assertioin > - Squash the spurious newline into the previous commit > v2: > - Zero out (*Target) in LsiScsiGetTargetLun() > - Use CopyMem() instead of the one-byte shortcut to copy target from > ScsiDevicePath->Pun > - Add asserts for PcdLsiScsiMaxTargetLimit and PcdLsiScsiMaxLunLimit > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Gary Lin > --- > OvmfPkg/LsiScsiDxe/LsiScsi.c | 148 +++++++++++++++++++++++++++++- > OvmfPkg/LsiScsiDxe/LsiScsi.h | 2 + > OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf | 6 ++ > OvmfPkg/OvmfPkg.dec | 5 + > 4 files changed, 159 insertions(+), 2 deletions(-) Reviewed-by: Laszlo Ersek Thanks, Laszlo > > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsi.c b/OvmfPkg/LsiScsiDxe/LsiScsi.c > index 67fadb411e85..172779240636 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsi.c > +++ b/OvmfPkg/LsiScsiDxe/LsiScsi.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -53,6 +54,49 @@ LsiScsiGetNextTargetLun ( > IN OUT UINT64 *Lun > ) > { > + LSI_SCSI_DEV *Dev; > + UINTN Idx; > + UINT8 *Target; > + UINT16 LastTarget; > + > + // > + // the TargetPointer input parameter is unnecessarily a pointer-to-pointer > + // > + Target = *TargetPointer; > + > + // > + // Search for first non-0xFF byte. If not found, return first target & LUN. > + // > + for (Idx = 0; Idx < TARGET_MAX_BYTES && Target[Idx] == 0xFF; ++Idx) > + ; > + if (Idx == TARGET_MAX_BYTES) { > + SetMem (Target, TARGET_MAX_BYTES, 0x00); > + *Lun = 0; > + return EFI_SUCCESS; > + } > + > + CopyMem (&LastTarget, Target, sizeof LastTarget); > + > + // > + // increment (target, LUN) pair if valid on input > + // > + Dev = LSI_SCSI_FROM_PASS_THRU (This); > + if (LastTarget > Dev->MaxTarget || *Lun > Dev->MaxLun) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (*Lun < Dev->MaxLun) { > + ++*Lun; > + return EFI_SUCCESS; > + } > + > + if (LastTarget < Dev->MaxTarget) { > + *Lun = 0; > + ++LastTarget; > + CopyMem (Target, &LastTarget, sizeof LastTarget); > + return EFI_SUCCESS; > + } > + > return EFI_NOT_FOUND; > } > > @@ -65,7 +109,34 @@ LsiScsiBuildDevicePath ( > IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath > ) > { > - return EFI_NOT_FOUND; > + UINT16 TargetValue; > + LSI_SCSI_DEV *Dev; > + SCSI_DEVICE_PATH *ScsiDevicePath; > + > + if (DevicePath == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + CopyMem (&TargetValue, Target, sizeof TargetValue); > + Dev = LSI_SCSI_FROM_PASS_THRU (This); > + if (TargetValue > Dev->MaxTarget || Lun > Dev->MaxLun || Lun > 0xFFFF) { > + return EFI_NOT_FOUND; > + } > + > + ScsiDevicePath = AllocatePool (sizeof *ScsiDevicePath); > + if (ScsiDevicePath == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + ScsiDevicePath->Header.Type = MESSAGING_DEVICE_PATH; > + ScsiDevicePath->Header.SubType = MSG_SCSI_DP; > + ScsiDevicePath->Header.Length[0] = (UINT8) sizeof *ScsiDevicePath; > + ScsiDevicePath->Header.Length[1] = (UINT8) (sizeof *ScsiDevicePath >> 8); > + ScsiDevicePath->Pun = TargetValue; > + ScsiDevicePath->Lun = (UINT16) Lun; > + > + *DevicePath = &ScsiDevicePath->Header; > + return EFI_SUCCESS; > } > > EFI_STATUS > @@ -77,7 +148,33 @@ LsiScsiGetTargetLun ( > OUT UINT64 *Lun > ) > { > - return EFI_UNSUPPORTED; > + SCSI_DEVICE_PATH *ScsiDevicePath; > + LSI_SCSI_DEV *Dev; > + UINT8 *Target; > + > + if (DevicePath == NULL || TargetPointer == NULL || *TargetPointer == NULL || > + Lun == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (DevicePath->Type != MESSAGING_DEVICE_PATH || > + DevicePath->SubType != MSG_SCSI_DP) { > + return EFI_UNSUPPORTED; > + } > + > + ScsiDevicePath = (SCSI_DEVICE_PATH *) DevicePath; > + Dev = LSI_SCSI_FROM_PASS_THRU (This); > + if (ScsiDevicePath->Pun > Dev->MaxTarget || > + ScsiDevicePath->Lun > Dev->MaxLun) { > + return EFI_NOT_FOUND; > + } > + > + Target = *TargetPointer; > + ZeroMem (Target, TARGET_MAX_BYTES); > + CopyMem (Target, &ScsiDevicePath->Pun, sizeof ScsiDevicePath->Pun); > + *Lun = ScsiDevicePath->Lun; > + > + return EFI_SUCCESS; > } > > EFI_STATUS > @@ -107,6 +204,42 @@ LsiScsiGetNextTarget ( > IN OUT UINT8 **TargetPointer > ) > { > + LSI_SCSI_DEV *Dev; > + UINTN Idx; > + UINT8 *Target; > + UINT16 LastTarget; > + > + // > + // the TargetPointer input parameter is unnecessarily a pointer-to-pointer > + // > + Target = *TargetPointer; > + > + // > + // Search for first non-0xFF byte. If not found, return first target. > + // > + for (Idx = 0; Idx < TARGET_MAX_BYTES && Target[Idx] == 0xFF; ++Idx) > + ; > + if (Idx == TARGET_MAX_BYTES) { > + SetMem (Target, TARGET_MAX_BYTES, 0x00); > + return EFI_SUCCESS; > + } > + > + CopyMem (&LastTarget, Target, sizeof LastTarget); > + > + // > + // increment target if valid on input > + // > + Dev = LSI_SCSI_FROM_PASS_THRU (This); > + if (LastTarget > Dev->MaxTarget) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (LastTarget < Dev->MaxTarget) { > + ++LastTarget; > + CopyMem (Target, &LastTarget, sizeof LastTarget); > + return EFI_SUCCESS; > + } > + > return EFI_NOT_FOUND; > } > > @@ -189,6 +322,17 @@ LsiScsiControllerStart ( > > Dev->Signature = LSI_SCSI_DEV_SIGNATURE; > > + STATIC_ASSERT ( > + FixedPcdGet8 (PcdLsiScsiMaxTargetLimit) < 8, > + "LSI 53C895A supports targets [0..7]" > + ); > + STATIC_ASSERT ( > + FixedPcdGet8 (PcdLsiScsiMaxLunLimit) < 128, > + "LSI 53C895A supports LUNs [0..127]" > + ); > + Dev->MaxTarget = PcdGet8 (PcdLsiScsiMaxTargetLimit); > + Dev->MaxLun = PcdGet8 (PcdLsiScsiMaxLunLimit); > + > // > // Host adapter channel, doesn't exist > // > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsi.h b/OvmfPkg/LsiScsiDxe/LsiScsi.h > index 751d5b193b51..6c6ed25f1c33 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsi.h > +++ b/OvmfPkg/LsiScsiDxe/LsiScsi.h > @@ -14,6 +14,8 @@ > > typedef struct { > UINT32 Signature; > + UINT8 MaxTarget; > + UINT8 MaxLun; > EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode; > EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru; > } LSI_SCSI_DEV; > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf b/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf > index 14f9c68308cc..6111449577a8 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf > +++ b/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf > @@ -27,7 +27,9 @@ [Packages] > [LibraryClasses] > BaseLib > BaseMemoryLib > + DebugLib > MemoryAllocationLib > + PcdLib > UefiBootServicesTableLib > UefiDriverEntryPoint > UefiLib > @@ -35,3 +37,7 @@ [LibraryClasses] > [Protocols] > gEfiExtScsiPassThruProtocolGuid ## BY_START > gEfiPciIoProtocolGuid ## TO_START > + > +[FixedPcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxTargetLimit ## CONSUMES > + gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxLunLimit ## CONSUMES > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index 2b0f137cbcce..ca13a3cb11d3 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -174,6 +174,11 @@ [PcdsFixedAtBuild] > ## Microseconds to stall between polling for MptScsi request result > gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiStallPerPollUsec|5|UINT32|0x3a > > + ## Set the *inclusive* number of targets and LUNs that LsiScsi exposes for > + # scan by ScsiBusDxe. > + gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxTargetLimit|7|UINT8|0x3b > + gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxLunLimit|0|UINT8|0x3c > + > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa >