From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [63.128.21.74]) by mx.groups.io with SMTP id smtpd.web12.3714.1585101460345688206 for ; Tue, 24 Mar 2020 18:57:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=c8i6J2Sa; spf=pass (domain: redhat.com, ip: 63.128.21.74, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1585101459; 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=B0GE1uBXAXwhkg2YQ6qQcBn2V6DIFekJKzB3vDz9JOs=; b=c8i6J2Sa4euysMRzkPOA/8qWEGSwul3LN0WNQ81AUoXKc83hTOarYHnGXWviPG56QKfFqr lHnkg2OghQlqAH6ufzkIqZ/iZw8uXx270VsHA5irMTx9LYjnxFIgGn+NBzf8O46w+v9NtM aqL+x4oV8RK80p4+KK+ZhSa1msjYSSI= 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-406-Pt5EqleCPO-qpoQ0XY4XZw-1; Tue, 24 Mar 2020 21:57:30 -0400 X-MC-Unique: Pt5EqleCPO-qpoQ0XY4XZw-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8103B1851C2D; Wed, 25 Mar 2020 01:57:29 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-139.ams2.redhat.com [10.36.115.139]) by smtp.corp.redhat.com (Postfix) with ESMTP id 76BFF5C241; Wed, 25 Mar 2020 01:57:27 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL To: devel@edk2.groups.io, michael.kubacki@outlook.com Cc: Bret Barkelew , Liming Gao , Michael D Kinney , Jian J Wang , Hao A Wu References: From: "Laszlo Ersek" Message-ID: Date: Wed, 25 Mar 2020 02:57:26 +0100 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.16 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 Hello Michael, On 03/24/20 23:04, Michael Kubacki wrote: > From: Michael Kubacki > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062 > > The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()" > "Attributes" parameter description states: > > "If not NULL, a pointer to the memory location to return the > attributes bitmask for the variable. See 'Related Definitions.' > If not NULL, then Attributes is set on output both when > EFI_SUCCESS and when EFI_BUFFER_TOO_SMALL is returned." > > The attributes were previously only returned from the implementation > in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or > EFI_BUFFER_TOO_SMALL according to spec. > > Cc: Bret Barkelew > Cc: Liming Gao > Cc: Michael D Kinney > Cc: Jian J Wang > Cc: Hao A Wu > Signed-off-by: Michael Kubacki > --- > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10 +++++++--- > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 10 ++++++---- > 2 files changed, 13 insertions(+), 7 deletions(-) per : Can you please turn this submission into a two-part patch series, and append a revert of commit 6896efdec270, as second patch? Thank you, Laszlo > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > index d23aea4bc712..1e71fc642c76 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c > @@ -18,6 +18,8 @@ > > Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
> (C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP
> +Copyright (c) Microsoft Corporation.
> + > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -2379,9 +2381,6 @@ VariableServiceGetVariable ( > } > > CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr, mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize); > - if (Attributes != NULL) { > - *Attributes = Variable.CurrPtr->Attributes; > - } > > *DataSize = VarDataSize; > UpdateVariableInfo (VariableName, VendorGuid, Variable.Volatile, TRUE, FALSE, FALSE, FALSE, &gVariableInfo); > @@ -2395,6 +2394,11 @@ VariableServiceGetVariable ( > } > > Done: > + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) { > + if (Attributes != NULL && Variable.CurrPtr != NULL) { > + *Attributes = Variable.CurrPtr->Attributes; > + } > + } > ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal->VariableGlobal.VariableServicesLock); > return Status; > } > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > index 2cf0ed32ae55..ca833fb0244d 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c > @@ -14,6 +14,7 @@ > InitCommunicateBuffer() is really function to check the variable data size. > > Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
> +Copyright (c) Microsoft Corporation.
> SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -642,10 +643,6 @@ FindVariableInRuntimeCache ( > } > > CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr, mVariableAuthFormat), TempDataSize); > - if (Attributes != NULL) { > - *Attributes = RtPtrTrack.CurrPtr->Attributes; > - } > - > *DataSize = TempDataSize; > > UpdateVariableInfo (VariableName, VendorGuid, RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, &mVariableInfo); > @@ -661,6 +658,11 @@ FindVariableInRuntimeCache ( > } > > Done: > + if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) { > + if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) { > + *Attributes = RtPtrTrack.CurrPtr->Attributes; > + } > + } > mVariableRuntimeCacheReadLock = FALSE; > > return Status; >