From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=104.47.1.57; helo=eur01-ve1-obe.outbound.protection.outlook.com; envelope-from=pankaj.bansal@nxp.com; receiver=edk2-devel@lists.01.org Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0057.outbound.protection.outlook.com [104.47.1.57]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9F02520954CDE for ; Mon, 19 Feb 2018 20:44:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=B0NBSIUZ+vrUSEnouMHUz/sKk27uJkVEJbAxKngfxG8=; b=AvdQWmpot1omrg2KT1dWeTY5vilgPiwtI54pJL4k40QA4ZfM1DmIe6geSDdFZ5s+n+xsjuSLSptDVLVXF23t43HRyjmZlsVLslkiRLNIgXx/oHQws90BExA56CEqWlFBbs8Xa3CloV3+VkF7D0KI8KQ2MHBk3DkwIzbdR+bkcIw= Received: from AM0PR0402MB3940.eurprd04.prod.outlook.com (52.133.40.140) by AM0PR0402MB3635.eurprd04.prod.outlook.com (52.133.38.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.485.10; Tue, 20 Feb 2018 04:49:56 +0000 Received: from AM0PR0402MB3940.eurprd04.prod.outlook.com ([fe80::4815:8101:2a92:25ea]) by AM0PR0402MB3940.eurprd04.prod.outlook.com ([fe80::4815:8101:2a92:25ea%13]) with mapi id 15.20.0506.020; Tue, 20 Feb 2018 04:49:55 +0000 From: Pankaj Bansal To: "edk2-devel@lists.01.org" CC: Liming Gao , Michael D Kinney Thread-Topic: [RFC] MdePkg/BaseLib: Change BitField functions. Thread-Index: AQHToZVOQewKy+TSDEeGa3mFQGqLrKOsyRvw Date: Tue, 20 Feb 2018 04:49:55 +0000 Message-ID: References: <1518173996-26906-1-git-send-email-pankaj.bansal@nxp.com> In-Reply-To: <1518173996-26906-1-git-send-email-pankaj.bansal@nxp.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=pankaj.bansal@nxp.com; x-originating-ip: [49.36.1.179] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM0PR0402MB3635; 7:Ne9b96x6A+/Se3IcwOOhiewCZNTdaxrGnEoVpB9njm//ySpEOLjf2aTYKlV6j8WG1Z+RIStfAT+2pQliRLik4ut+Suo4q2YbGLmoibM48YV/zALJnyzEu8LDb/W9DfN9PHOMw+U1iEZo0Lc9co82K/pNd08agF00zl3+goOsJwDBHHZWyaz8MTMzCGsd91JR0wVKYnQGUlf1TZCybDqIjf6M38bd785SiQ94IL0PmYPSTQSZsrhB8ZgZlgGvu8WP x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 477c4627-2504-4087-83a4-08d5781d635c x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(3008032)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603307)(7153060)(7193020); SRVR:AM0PR0402MB3635; x-ms-traffictypediagnostic: AM0PR0402MB3635: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(185117386973197)(162533806227266)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001056)(6040501)(2401047)(5005006)(8121501046)(93006095)(93001095)(3002001)(3231101)(944501161)(10201501046)(6055026)(6041288)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123564045)(20161123560045)(20161123562045)(6072148)(201708071742011); SRVR:AM0PR0402MB3635; BCL:0; PCL:0; RULEID:; SRVR:AM0PR0402MB3635; x-forefront-prvs: 05891FB07F x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(346002)(376002)(39380400002)(39860400002)(396003)(13464003)(199004)(189003)(53754006)(33656002)(3280700002)(7736002)(6246003)(5250100002)(229853002)(2351001)(3660700001)(2501003)(106356001)(53546011)(14454004)(2900100001)(97736004)(25786009)(68736007)(305945005)(74316002)(26005)(6916009)(2950100002)(5660300001)(102836004)(8676002)(186003)(6116002)(7696005)(81166006)(105586002)(86362001)(2906002)(54906003)(53936002)(66066001)(99286004)(5640700003)(3846002)(81156014)(4326008)(55016002)(6506007)(76176011)(8936002)(6436002)(478600001)(9686003)(316002)(19627235001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR0402MB3635; H:AM0PR0402MB3940.eurprd04.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: DxYoncr9I4WOCWmb5xWUe0/d4lVDGnt80z/7/Q8PQyMHxreenoKYxRm+xk22+4OtHRakZw4Ngz1hU0Oqd0ns3iUxn/o0/kbka4VVNJS5WRJ9D6uLityty66tO1X4HtGN1JcCTkBdtrHIBXBgJ45Qf/TCpaoFRNB2MPdOH6rUviY= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 477c4627-2504-4087-83a4-08d5781d635c X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Feb 2018 04:49:55.7935 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR0402MB3635 Subject: Re: [RFC] MdePkg/BaseLib: Change BitField functions. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Feb 2018 04:44:02 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi everybody, Any comments on this change ? > -----Original Message----- > From: Pankaj Bansal > Sent: Friday, February 09, 2018 4:30 PM > To: edk2-devel@lists.01.org > Cc: Pankaj Bansal ; Michael D Kinney > ; Liming Gao > Subject: [RFC] MdePkg/BaseLib: Change BitField functions. >=20 > The bit field functions in MdePkg are not working as expected. > The restrictions on Value parameter such that Value should not be greater > than the bitmask value range specified by StartBit and EndBit doesn't see= m to > make sense. >=20 > Also the restriction on End bit to not be equal to start bit prohibits si= ngle bit > change. >=20 > This is an attempt to correct these limitations. >=20 > Cc: Michael D Kinney > Cc: Liming Gao > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Pankaj Bansal > --- >=20 > Notes: > This is a RFC patch. >=20 > MdePkg/Library/BaseLib/BitField.c | 179 ++++++++++++++-------------- > 1 file changed, 89 insertions(+), 90 deletions(-) >=20 > diff --git a/MdePkg/Library/BaseLib/BitField.c > b/MdePkg/Library/BaseLib/BitField.c > index eb9e276..2b5be52 100644 > --- a/MdePkg/Library/BaseLib/BitField.c > +++ b/MdePkg/Library/BaseLib/BitField.c > @@ -2,6 +2,8 @@ > Bit field functions of BaseLib. >=20 > Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
> + Copyright 2018 NXP > + > 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 @@ -14,6 +16,12 @@ >=20 > #include "BaseLibInternals.h" >=20 > +#define BITMASK_UNITN(StartBit, EndBit) \ > + (((MAX_UINTN) << (StartBit)) & (MAX_UINTN >> ((sizeof (UINTN) * 8) > +- 1 - (EndBit)))) > + > +#define BITMASK_UNIT64(StartBit, EndBit) \ > + (((MAX_UINT64) << (StartBit)) & (MAX_UINT64 >> ((sizeof (UINT64) * > +8) - 1 - (EndBit)))) > + > /** > Worker function that returns a bit field from Operand. >=20 > @@ -34,11 +42,9 @@ InternalBaseLibBitFieldReadUint ( > IN UINTN EndBit > ) > { > - // > - // ~((UINTN)-2 << EndBit) is a mask in which bit[0] thru bit[EndBit] > - // are 1's while bit[EndBit + 1] thru the most significant bit are 0's= . > - // > - return (Operand & ~((UINTN)-2 << EndBit)) >> StartBit; > + UINTN Mask =3D BITMASK_UNITN (StartBit, EndBit); > + > + return ( (Operand & Mask) >> StartBit); > } >=20 > /** > @@ -68,19 +74,9 @@ InternalBaseLibBitFieldOrUint ( > IN UINTN OrData > ) > { > - // > - // Higher bits in OrData those are not used must be zero. > - // > - // EndBit - StartBit + 1 might be 32 while the result right shifting 3= 2 on a > 32bit integer is undefined, > - // So the logic is updated to right shift (EndBit - StartBit) bits and= compare > the last bit directly. > - // > - ASSERT ((OrData >> (EndBit - StartBit)) =3D=3D ((OrData >> (EndBit - S= tartBit)) & > 1)); > - > - // > - // ~((UINTN)-2 << EndBit) is a mask in which bit[0] thru bit[EndBit] > - // are 1's while bit[EndBit + 1] thru the most significant bit are 0's= . > - // > - return Operand | ((OrData << StartBit) & ~((UINTN) -2 << EndBit)); > + UINTN Mask =3D BITMASK_UNITN (StartBit, EndBit); > + > + return ( ( (Operand | OrData) & Mask) | (Operand & ~Mask)); > } >=20 > /** > @@ -110,19 +106,38 @@ InternalBaseLibBitFieldAndUint ( > IN UINTN AndData > ) > { > - // > - // Higher bits in AndData those are not used must be zero. > - // > - // EndBit - StartBit + 1 might be 32 while the result right shifting 3= 2 on a > 32bit integer is undefined, > - // So the logic is updated to right shift (EndBit - StartBit) bits and= compare > the last bit directly. > - // > - ASSERT ((AndData >> (EndBit - StartBit)) =3D=3D ((AndData >> (EndBit -= StartBit)) > & 1)); > - > - // > - // ~((UINTN)-2 << EndBit) is a mask in which bit[0] thru bit[EndBit] > - // are 1's while bit[EndBit + 1] thru the most significant bit are 0's= . > - // > - return Operand & ~((~AndData << StartBit) & ~((UINTN)-2 << EndBit)); > + UINTN Mask =3D BITMASK_UNITN (StartBit, EndBit); > + > + return ( ( (Operand & AndData) & Mask) | (Operand & ~Mask)); > +} > + > +/** > + Worker function that writes a bit field to an value, and returns the r= esult. > + > + Writes Value to the bit field specified by the StartBit and the EndBit= in > + Operand. All other bits in Operand are preserved. The new 8-bit value = is > + returned. > + > + @param Operand Operand on which to perform the bitfield operation. > + @param StartBit The ordinal of the least significant bit in the bit = field. > + @param EndBit The ordinal of the most significant bit in the bit f= ield. > + @param Value The new value of the bit field. > + > + @return The new value. > + > +**/ > +UINTN > +EFIAPI > +InternalBaseLibBitFieldWriteUint ( > + IN UINTN Operand, > + IN UINTN StartBit, > + IN UINTN EndBit, > + IN UINTN Value > + ) > +{ > + UINTN Mask =3D BITMASK_UNITN (StartBit, EndBit); > + > + return ( (Value & Mask) | (Operand & ~Mask)); > } >=20 > /** > @@ -153,7 +168,7 @@ BitFieldRead8 ( > ) > { > ASSERT (EndBit < 8); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT8)InternalBaseLibBitFieldReadUint (Operand, StartBit, EndB= it); > } >=20 > @@ -190,8 +205,8 @@ BitFieldWrite8 ( > ) > { > ASSERT (EndBit < 8); > - ASSERT (StartBit <=3D EndBit); > - return BitFieldAndThenOr8 (Operand, StartBit, EndBit, 0, Value); > + ASSERT (StartBit < EndBit); > + return (UINT8)InternalBaseLibBitFieldWriteUint (Operand, StartBit, End= Bit, > Value); > } >=20 > /** > @@ -228,7 +243,7 @@ BitFieldOr8 ( > ) > { > ASSERT (EndBit < 8); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT8)InternalBaseLibBitFieldOrUint (Operand, StartBit, EndBit= , > OrData); > } >=20 > @@ -266,7 +281,7 @@ BitFieldAnd8 ( > ) > { > ASSERT (EndBit < 8); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT8)InternalBaseLibBitFieldAndUint (Operand, StartBit, EndBi= t, > AndData); > } >=20 > @@ -275,7 +290,7 @@ BitFieldAnd8 ( > bitwise OR, and returns the result. >=20 > Performs a bitwise AND between the bit field specified by StartBit and > EndBit > - in Operand and the value specified by AndData, followed by a bitwise > + in Operand and the value specified by AndData, followed by a bitwise > OR with value specified by OrData. All other bits in Operand are > preserved. The new 8-bit value is returned. >=20 > @@ -308,7 +323,7 @@ BitFieldAndThenOr8 ( > ) > { > ASSERT (EndBit < 8); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return BitFieldOr8 ( > BitFieldAnd8 (Operand, StartBit, EndBit, AndData), > StartBit, > @@ -345,7 +360,7 @@ BitFieldRead16 ( > ) > { > ASSERT (EndBit < 16); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT16)InternalBaseLibBitFieldReadUint (Operand, StartBit, End= Bit); > } >=20 > @@ -382,8 +397,8 @@ BitFieldWrite16 ( > ) > { > ASSERT (EndBit < 16); > - ASSERT (StartBit <=3D EndBit); > - return BitFieldAndThenOr16 (Operand, StartBit, EndBit, 0, Value); > + ASSERT (StartBit < EndBit); > + return (UINT16)InternalBaseLibBitFieldWriteUint (Operand, StartBit, > EndBit, Value); > } >=20 > /** > @@ -420,7 +435,7 @@ BitFieldOr16 ( > ) > { > ASSERT (EndBit < 16); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT16)InternalBaseLibBitFieldOrUint (Operand, StartBit, EndBi= t, > OrData); > } >=20 > @@ -458,7 +473,7 @@ BitFieldAnd16 ( > ) > { > ASSERT (EndBit < 16); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT16)InternalBaseLibBitFieldAndUint (Operand, StartBit, EndB= it, > AndData); > } >=20 > @@ -467,7 +482,7 @@ BitFieldAnd16 ( > bitwise OR, and returns the result. >=20 > Performs a bitwise AND between the bit field specified by StartBit and > EndBit > - in Operand and the value specified by AndData, followed by a bitwise > + in Operand and the value specified by AndData, followed by a bitwise > OR with value specified by OrData. All other bits in Operand are > preserved. The new 16-bit value is returned. >=20 > @@ -500,7 +515,7 @@ BitFieldAndThenOr16 ( > ) > { > ASSERT (EndBit < 16); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return BitFieldOr16 ( > BitFieldAnd16 (Operand, StartBit, EndBit, AndData), > StartBit, > @@ -537,7 +552,7 @@ BitFieldRead32 ( > ) > { > ASSERT (EndBit < 32); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT32)InternalBaseLibBitFieldReadUint (Operand, StartBit, End= Bit); > } >=20 > @@ -574,8 +589,8 @@ BitFieldWrite32 ( > ) > { > ASSERT (EndBit < 32); > - ASSERT (StartBit <=3D EndBit); > - return BitFieldAndThenOr32 (Operand, StartBit, EndBit, 0, Value); > + ASSERT (StartBit < EndBit); > + return (UINT32)InternalBaseLibBitFieldWriteUint (Operand, StartBit, > EndBit, Value); > } >=20 > /** > @@ -612,7 +627,7 @@ BitFieldOr32 ( > ) > { > ASSERT (EndBit < 32); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT32)InternalBaseLibBitFieldOrUint (Operand, StartBit, EndBi= t, > OrData); > } >=20 > @@ -650,7 +665,7 @@ BitFieldAnd32 ( > ) > { > ASSERT (EndBit < 32); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return (UINT32)InternalBaseLibBitFieldAndUint (Operand, StartBit, EndB= it, > AndData); > } >=20 > @@ -659,7 +674,7 @@ BitFieldAnd32 ( > bitwise OR, and returns the result. >=20 > Performs a bitwise AND between the bit field specified by StartBit and > EndBit > - in Operand and the value specified by AndData, followed by a bitwise > + in Operand and the value specified by AndData, followed by a bitwise > OR with value specified by OrData. All other bits in Operand are > preserved. The new 32-bit value is returned. >=20 > @@ -692,7 +707,7 @@ BitFieldAndThenOr32 ( > ) > { > ASSERT (EndBit < 32); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return BitFieldOr32 ( > BitFieldAnd32 (Operand, StartBit, EndBit, AndData), > StartBit, > @@ -729,8 +744,11 @@ BitFieldRead64 ( > ) > { > ASSERT (EndBit < 64); > - ASSERT (StartBit <=3D EndBit); > - return RShiftU64 (Operand & ~LShiftU64 ((UINT64)-2, EndBit), StartBit)= ; > + ASSERT (StartBit < EndBit); > + > + UINT64 Mask =3D BITMASK_UNIT64 (StartBit, EndBit); > + > + return ( (Operand & Mask) >> StartBit); > } >=20 > /** > @@ -766,8 +784,11 @@ BitFieldWrite64 ( > ) > { > ASSERT (EndBit < 64); > - ASSERT (StartBit <=3D EndBit); > - return BitFieldAndThenOr64 (Operand, StartBit, EndBit, 0, Value); > + ASSERT (StartBit < EndBit); > + > + UINT64 Mask =3D BITMASK_UNIT64 (StartBit, EndBit); > + > + return ( (Value & Mask) | (Operand & ~Mask)); > } >=20 > /** > @@ -803,23 +824,12 @@ BitFieldOr64 ( > IN UINT64 OrData > ) > { > - UINT64 Value1; > - UINT64 Value2; > - > ASSERT (EndBit < 64); > - ASSERT (StartBit <=3D EndBit); > - // > - // Higher bits in OrData those are not used must be zero. > - // > - // EndBit - StartBit + 1 might be 64 while the result right shifting 6= 4 on > RShiftU64() API is invalid, > - // So the logic is updated to right shift (EndBit - StartBit) bits and= compare > the last bit directly. > - // > - ASSERT (RShiftU64 (OrData, EndBit - StartBit) =3D=3D (RShiftU64 (OrDat= a, EndBit > - StartBit) & 1)); > - > - Value1 =3D LShiftU64 (OrData, StartBit); > - Value2 =3D LShiftU64 ((UINT64) - 2, EndBit); > - > - return Operand | (Value1 & ~Value2); > + ASSERT (StartBit < EndBit); > + > + UINT64 Mask =3D BITMASK_UNIT64 (StartBit, EndBit); > + > + return ( ( (Operand | OrData) & Mask) | (Operand & ~Mask)); > } >=20 > /** > @@ -855,23 +865,12 @@ BitFieldAnd64 ( > IN UINT64 AndData > ) > { > - UINT64 Value1; > - UINT64 Value2; > - > ASSERT (EndBit < 64); > - ASSERT (StartBit <=3D EndBit); > - // > - // Higher bits in AndData those are not used must be zero. > - // > - // EndBit - StartBit + 1 might be 64 while the right shifting 64 on RS= hiftU64() > API is invalid, > - // So the logic is updated to right shift (EndBit - StartBit) bits and= compare > the last bit directly. > - // > - ASSERT (RShiftU64 (AndData, EndBit - StartBit) =3D=3D (RShiftU64 (AndD= ata, > EndBit - StartBit) & 1)); > - > - Value1 =3D LShiftU64 (~AndData, StartBit); > - Value2 =3D LShiftU64 ((UINT64)-2, EndBit); > - > - return Operand & ~(Value1 & ~Value2); > + ASSERT (StartBit < EndBit); > + > + UINT64 Mask =3D BITMASK_UNIT64 (StartBit, EndBit); > + > + return ( ( (Operand & AndData) & Mask) | (Operand & ~Mask)); > } >=20 > /** > @@ -879,7 +878,7 @@ BitFieldAnd64 ( > bitwise OR, and returns the result. >=20 > Performs a bitwise AND between the bit field specified by StartBit and > EndBit > - in Operand and the value specified by AndData, followed by a bitwise > + in Operand and the value specified by AndData, followed by a bitwise > OR with value specified by OrData. All other bits in Operand are > preserved. The new 64-bit value is returned. >=20 > @@ -912,7 +911,7 @@ BitFieldAndThenOr64 ( > ) > { > ASSERT (EndBit < 64); > - ASSERT (StartBit <=3D EndBit); > + ASSERT (StartBit < EndBit); > return BitFieldOr64 ( > BitFieldAnd64 (Operand, StartBit, EndBit, AndData), > StartBit, > -- > 2.7.4