From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=ray.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C65CA209574CA for ; Mon, 25 Feb 2019 00:05:03 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Feb 2019 00:05:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,410,1544515200"; d="scan'208";a="149699290" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga001.fm.intel.com with ESMTP; 25 Feb 2019 00:05:03 -0800 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 25 Feb 2019 00:05:02 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx117.amr.corp.intel.com (10.18.116.17) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 25 Feb 2019 00:05:02 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.102]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.172]) with mapi id 14.03.0415.000; Mon, 25 Feb 2019 16:05:00 +0800 From: "Ni, Ray" To: "Wu, Hao A" , "edk2-devel@lists.01.org" CC: "Wu, Hao A" Thread-Topic: [edk2] [PATCH v1] MdeModulePkg/UfsBlockIoPei: Correct use of 'DeviceIndex' in BlkIO PPI Thread-Index: AQHUxNnoyVo7mBZkPEOnoZXX1BHt3aXwN69Q Date: Mon, 25 Feb 2019 08:01:29 +0000 Deferred-Delivery: Mon, 25 Feb 2019 08:05:00 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C030BB7@SHSMSX104.ccr.corp.intel.com> References: <20190215025513.19540-1-hao.a.wu@intel.com> In-Reply-To: <20190215025513.19540-1-hao.a.wu@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v1] MdeModulePkg/UfsBlockIoPei: Correct use of 'DeviceIndex' in BlkIO PPI X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Feb 2019 08:05:04 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ray Ni > -----Original Message----- > From: edk2-devel On Behalf Of Hao Wu > Sent: Friday, February 15, 2019 10:55 AM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A > Subject: [edk2] [PATCH v1] MdeModulePkg/UfsBlockIoPei: Correct use of > 'DeviceIndex' in BlkIO PPI >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D1474 >=20 > Within UfsBlockIoPei, the current implementation of the Block IO(2) > services: >=20 > UfsBlockIoPeimGetMediaInfo > UfsBlockIoPeimReadBlocks > UfsBlockIoPeimGetMediaInfo2 > UfsBlockIoPeimReadBlocks2 >=20 > does not handle the input parameter 'DeviceIndex' properly. >=20 > According to both of the PI spec and the function description comments: >=20 > > DeviceIndex Specifies the block device to which the function wants > > to talk. ... This index is a number from one to > > NumberBlockDevices. >=20 > But current codes incorrectly treat the valid range of 'DeviceIndex' as 0= to > (NumberBlockDevices - 1). >=20 > This commit is to address this issue. >=20 > Cc: Jian J Wang > Cc: Ray Ni > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Hao Wu > --- > MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 56 +++++++++++- > -------- > 1 file changed, 31 insertions(+), 25 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c > b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c > index 204e456413..c969ab45f5 100644 > --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c > +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c > @@ -1,6 +1,6 @@ > /** @file >=20 > - Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2014 - 2019, Intel Corporation. All rights > + reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be > found at @@ -587,15 +587,17 @@ UfsBlockIoPeimGetMediaInfo ( > EFI_SCSI_DISK_CAPACITY_DATA16 Capacity16; > UINTN DataLength; > BOOLEAN NeedRetry; > + UINTN Lun; >=20 > Private =3D GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS (This); > NeedRetry =3D TRUE; >=20 > - if (DeviceIndex >=3D UFS_PEIM_MAX_LUNS) { > + if ((DeviceIndex =3D=3D 0) || (DeviceIndex > UFS_PEIM_MAX_LUNS)) { > return EFI_INVALID_PARAMETER; > } >=20 > - if ((Private->Luns.BitMask & (BIT0 << DeviceIndex)) =3D=3D 0) { > + Lun =3D DeviceIndex - 1; > + if ((Private->Luns.BitMask & (BIT0 << Lun)) =3D=3D 0) { > return EFI_ACCESS_DENIED; > } >=20 > @@ -609,7 +611,7 @@ UfsBlockIoPeimGetMediaInfo ( > do { > Status =3D UfsPeimTestUnitReady ( > Private, > - DeviceIndex, > + Lun, > &SenseData, > &SenseDataLength > ); > @@ -621,7 +623,7 @@ UfsBlockIoPeimGetMediaInfo ( > continue; > } >=20 > - Status =3D UfsPeimParsingSenseKeys (&(Private->Media[DeviceIndex]), > &SenseData, &NeedRetry); > + Status =3D UfsPeimParsingSenseKeys (&(Private->Media[Lun]), > + &SenseData, &NeedRetry); > if (EFI_ERROR (Status)) { > return EFI_DEVICE_ERROR; > } > @@ -630,7 +632,7 @@ UfsBlockIoPeimGetMediaInfo ( >=20 > DataLength =3D sizeof (EFI_SCSI_DISK_CAPACITY_DATA); > SenseDataLength =3D 0; > - Status =3D UfsPeimReadCapacity (Private, DeviceIndex, &Capacity, (UINT= 32 > *)&DataLength, NULL, &SenseDataLength); > + Status =3D UfsPeimReadCapacity (Private, Lun, &Capacity, (UINT32 > + *)&DataLength, NULL, &SenseDataLength); > if (EFI_ERROR (Status)) { > return EFI_DEVICE_ERROR; > } > @@ -639,22 +641,22 @@ UfsBlockIoPeimGetMediaInfo ( > (Capacity.LastLba1 =3D=3D 0xff) && (Capacity.LastLba0 =3D=3D 0xff)= ) { > DataLength =3D sizeof (EFI_SCSI_DISK_CAPACITY_DATA16); > SenseDataLength =3D 0; > - Status =3D UfsPeimReadCapacity16 (Private, DeviceIndex, &Capacity16, > (UINT32 *)&DataLength, NULL, &SenseDataLength); > + Status =3D UfsPeimReadCapacity16 (Private, Lun, &Capacity16, (UINT32 > + *)&DataLength, NULL, &SenseDataLength); > if (EFI_ERROR (Status)) { > return EFI_DEVICE_ERROR; > } > - Private->Media[DeviceIndex].LastBlock =3D ((UINT32)Capacity16.LastL= ba3 > << 24) | (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | > Capacity16.LastLba0; > - Private->Media[DeviceIndex].LastBlock |=3D LShiftU64 > ((UINT64)Capacity16.LastLba7, 56) | LShiftU64((UINT64)Capacity16.LastLba6= , > 48) | LShiftU64 ((UINT64)Capacity16.LastLba5, 40) | LShiftU64 > ((UINT64)Capacity16.LastLba4, 32); > - Private->Media[DeviceIndex].BlockSize =3D (Capacity16.BlockSize3 <<= 24) | > (Capacity16.BlockSize2 << 16) | (Capacity16.BlockSize1 << 8) | > Capacity16.BlockSize0; > + Private->Media[Lun].LastBlock =3D ((UINT32)Capacity16.LastLba3 << 2= 4) | > (Capacity16.LastLba2 << 16) | (Capacity16.LastLba1 << 8) | > Capacity16.LastLba0; > + Private->Media[Lun].LastBlock |=3D LShiftU64 ((UINT64)Capacity16.Las= tLba7, > 56) | LShiftU64((UINT64)Capacity16.LastLba6, 48) | LShiftU64 > ((UINT64)Capacity16.LastLba5, 40) | LShiftU64 ((UINT64)Capacity16.LastLba= 4, > 32); > + Private->Media[Lun].BlockSize =3D (Capacity16.BlockSize3 << 24) | > + (Capacity16.BlockSize2 << 16) | (Capacity16.BlockSize1 << 8) | > + Capacity16.BlockSize0; > } else { > - Private->Media[DeviceIndex].LastBlock =3D ((UINT32)Capacity.LastLba= 3 << > 24) | (Capacity.LastLba2 << 16) | (Capacity.LastLba1 << 8) | Capacity.Las= tLba0; > - Private->Media[DeviceIndex].BlockSize =3D (Capacity.BlockSize3 << 2= 4) | > (Capacity.BlockSize2 << 16) | (Capacity.BlockSize1 << 8) | Capacity.Block= Size0; > + Private->Media[Lun].LastBlock =3D ((UINT32)Capacity.LastLba3 << 24)= | > (Capacity.LastLba2 << 16) | (Capacity.LastLba1 << 8) | Capacity.LastLba0; > + Private->Media[Lun].BlockSize =3D (Capacity.BlockSize3 << 24) | > + (Capacity.BlockSize2 << 16) | (Capacity.BlockSize1 << 8) | > + Capacity.BlockSize0; > } >=20 > MediaInfo->DeviceType =3D UfsDevice; > - MediaInfo->MediaPresent =3D Private->Media[DeviceIndex].MediaPresent; > - MediaInfo->LastBlock =3D (UINTN)Private->Media[DeviceIndex].LastBlo= ck; > - MediaInfo->BlockSize =3D Private->Media[DeviceIndex].BlockSize; > + MediaInfo->MediaPresent =3D Private->Media[Lun].MediaPresent; > + MediaInfo->LastBlock =3D (UINTN)Private->Media[Lun].LastBlock; > + MediaInfo->BlockSize =3D Private->Media[Lun].BlockSize; >=20 > return EFI_SUCCESS; > } > @@ -711,6 +713,7 @@ UfsBlockIoPeimReadBlocks ( > EFI_SCSI_SENSE_DATA SenseData; > UINT8 SenseDataLength; > BOOLEAN NeedRetry; > + UINTN Lun; >=20 > Status =3D EFI_SUCCESS; > NeedRetry =3D TRUE; > @@ -730,21 +733,22 @@ UfsBlockIoPeimReadBlocks ( > return EFI_SUCCESS; > } >=20 > - if (DeviceIndex >=3D UFS_PEIM_MAX_LUNS) { > + if ((DeviceIndex =3D=3D 0) || (DeviceIndex > UFS_PEIM_MAX_LUNS)) { > return EFI_INVALID_PARAMETER; > } >=20 > - if ((Private->Luns.BitMask & (BIT0 << DeviceIndex)) =3D=3D 0) { > + Lun =3D DeviceIndex - 1; > + if ((Private->Luns.BitMask & (BIT0 << Lun)) =3D=3D 0) { > return EFI_ACCESS_DENIED; > } >=20 > - BlockSize =3D Private->Media[DeviceIndex].BlockSize; > + BlockSize =3D Private->Media[Lun].BlockSize; >=20 > if (BufferSize % BlockSize !=3D 0) { > Status =3D EFI_BAD_BUFFER_SIZE; > } >=20 > - if (StartLBA > Private->Media[DeviceIndex].LastBlock) { > + if (StartLBA > Private->Media[Lun].LastBlock) { > Status =3D EFI_INVALID_PARAMETER; > } >=20 > @@ -753,7 +757,7 @@ UfsBlockIoPeimReadBlocks ( > do { > Status =3D UfsPeimTestUnitReady ( > Private, > - DeviceIndex, > + Lun, > &SenseData, > &SenseDataLength > ); > @@ -765,7 +769,7 @@ UfsBlockIoPeimReadBlocks ( > continue; > } >=20 > - Status =3D UfsPeimParsingSenseKeys (&(Private->Media[DeviceIndex]), > &SenseData, &NeedRetry); > + Status =3D UfsPeimParsingSenseKeys (&(Private->Media[Lun]), > + &SenseData, &NeedRetry); > if (EFI_ERROR (Status)) { > return EFI_DEVICE_ERROR; > } > @@ -773,10 +777,10 @@ UfsBlockIoPeimReadBlocks ( > } while (NeedRetry); >=20 > SenseDataLength =3D 0; > - if (Private->Media[DeviceIndex].LastBlock < 0xfffffffful) { > + if (Private->Media[Lun].LastBlock < 0xfffffffful) { > Status =3D UfsPeimRead10 ( > Private, > - DeviceIndex, > + Lun, > (UINT32)StartLBA, > (UINT32)NumberOfBlocks, > Buffer, > @@ -787,7 +791,7 @@ UfsBlockIoPeimReadBlocks ( > } else { > Status =3D UfsPeimRead16 ( > Private, > - DeviceIndex, > + Lun, > (UINT32)StartLBA, > (UINT32)NumberOfBlocks, > Buffer, > @@ -888,6 +892,7 @@ UfsBlockIoPeimGetMediaInfo2 ( > EFI_STATUS Status; > UFS_PEIM_HC_PRIVATE_DATA *Private; > EFI_PEI_BLOCK_IO_MEDIA Media; > + UINTN Lun; >=20 > Private =3D GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS2 (This); >=20 > @@ -901,7 +906,8 @@ UfsBlockIoPeimGetMediaInfo2 ( > return Status; > } >=20 > - CopyMem (MediaInfo, &(Private->Media[DeviceIndex]), sizeof > (EFI_PEI_BLOCK_IO2_MEDIA)); > + Lun =3D DeviceIndex - 1; > + CopyMem (MediaInfo, &(Private->Media[Lun]), sizeof > + (EFI_PEI_BLOCK_IO2_MEDIA)); > return EFI_SUCCESS; > } >=20 > -- > 2.12.0.windows.1 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel