From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.2467.1587630122985143206 for ; Thu, 23 Apr 2020 01:22:03 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=S9TFK9Cc; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587630122; 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=Viz0C9j5LUwy8qrTyERL/5x8oQ3LJt/wVa4he63qVN0=; b=S9TFK9CcX/kcLRGHZh2+qpRyOoezgHw1vLU9zH5ClK9oUFou3T+rbY22csNR8H0zg/Ibjt XUcHBPjUb6D4Pi8KJUPSp7T/H/SuC40cMVgoebanHDpC5Gsx0W8RuXdAM/sVqVBNGbxWkA r9U2ZTxUNGTv+rNG6MakdgqBwPEvdpI= 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-416-bnurnN-oPfuZ2fOAkpfFfg-1; Thu, 23 Apr 2020 04:21:58 -0400 X-MC-Unique: bnurnN-oPfuZ2fOAkpfFfg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9D26B107ACC4; Thu, 23 Apr 2020 08:21:56 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-95.ams2.redhat.com [10.36.114.95]) by smtp.corp.redhat.com (Postfix) with ESMTP id 82C985DA82; Thu, 23 Apr 2020 08:21:54 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 2/6] OvmfPkg: Add QemuFwCfgLibNull To: devel@edk2.groups.io, rebecca@bsdio.com Cc: Jordan Justen , Ard Biesheuvel , Leif Lindholm , Michael Kinney , Andrew Fish , Peter Grehan References: <20200421030955.114850-1-rebecca@bsdio.com> <20200421030955.114850-3-rebecca@bsdio.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 23 Apr 2020 10:21:53 +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: <20200421030955.114850-3-rebecca@bsdio.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 04/21/20 05:09, Rebecca Cran wrote: > Add a null implementation library for QemuFwCfgLib, in order to > support building PciHostBridgeLib for bhyve. > > Signed-off-by: Rebecca Cran > --- > .../Library/QemuFwCfgLib/QemuFwCfgLibNull.inf | 37 ++++ > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c | 209 ++++++++++++++++++ > 2 files changed, 246 insertions(+) > create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf > create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c > > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf > new file mode 100644 > index 0000000000..09f86c2b02 > --- /dev/null > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibNull.inf > @@ -0,0 +1,37 @@ > +## @file > +# > +# Stateful, implicitly initialized fw_cfg library. (1) This comment should be updated -- it should say (for example): "Null implementation of the fw_cfg library". (I could update this for you myself, but below there is another change that I cannot do for you.) > +# > +# Copyright (C) 2013, Red Hat, Inc. > +# Copyright (c) 2008 - 2012, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.
(2) This is a new file; you should add your (C) on top (like you do in patch#1). I can't add this for you. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = QemuFwCfgLibNull > + FILE_GUID = B9D1A1F2-01E2-4732-982D-C7F9ED51AC6B > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = QemuFwCfgLib > + > +# > +# The following information is for reference only and not required by the build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 > +# (3) Technically speaking, the null instance is suitable for all architectures supported by edk2. I suggest simply dropping the VALID_ARCHITECTURES comment block. > + > +[Sources] > + QemuFwCfgLibInternal.h (4) We don't depend on this header file, please don't list it here. > + QemuFwCfgNull.c > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec (5) I *think* MdeModulePkg.dec is not needed, as we don't consume any content from MdeModulePkg. (The DebugLib class comes from MdePkg.) > + OvmfPkg/OvmfPkg.dec Yes, this is necessary (because the lib class header that we'll include comes from OvmfPkg). > + > +[LibraryClasses] > + DebugLib > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c > new file mode 100644 > index 0000000000..e2cc5f3406 > --- /dev/null > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgNull.c > @@ -0,0 +1,209 @@ > +/** @file > + > + Stateful and implicitly initialized fw_cfg library implementation. (6) Same as (1). > + > + Copyright (C) 2013, Red Hat, Inc. > + Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
> + Copyright (c) 2017, Advanced Micro Devices. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ (7) Same as (2). > + > +#include > +#include > +#include > + (8) I believe and can be dropped. In exchange, should be included. That's because the lib class header declares the functions that this lib instance will define (= implement). The lib class header also pulls in all dependencies that are needed for the API declarations. When you include , please add it under DebugLib.h, to keep the #include list sorted. > +/** > + Returns a boolean indicating if the firmware configuration interface > + is available or not. > + > + This function may change fw_cfg state. > + > + @retval TRUE The interface is available > + @retval FALSE The interface is not available > + > +**/ > +BOOLEAN > +EFIAPI > +QemuFwCfgIsAvailable ( > + VOID > + ) > +{ > + return FALSE; > +} > + > + > +/** > + Selects a firmware configuration item for reading. > + > + Following this call, any data read from this item will start from > + the beginning of the configuration item's data. > + > + @param[in] QemuFwCfgItem - Firmware Configuration item to read > + > +**/ > +VOID > +EFIAPI > +QemuFwCfgSelectItem ( > + IN FIRMWARE_CONFIG_ITEM QemuFwCfgItem > + ) > +{ > + ASSERT (FALSE); > +} > + > + > +/** > + Reads firmware configuration bytes into a buffer > + > + If called multiple times, then the data read will > + continue at the offset of the firmware configuration > + item where the previous read ended. > + > + @param[in] Size - Size in bytes to read > + @param[in] Buffer - Buffer to store data into > + > +**/ > +VOID > +EFIAPI > +QemuFwCfgReadBytes ( > + IN UINTN Size, > + IN VOID *Buffer OPTIONAL > + ) > +{ > + ASSERT (FALSE); > +} > + > + > +/** > + Writes firmware configuration bytes from a buffer > + > + If called multiple times, then the data written will > + continue at the offset of the firmware configuration > + item where the previous write ended. > + > + @param[in] Size - Size in bytes to write > + @param[in] Buffer - Buffer to read data from > + > +**/ > +VOID > +EFIAPI > +QemuFwCfgWriteBytes ( > + IN UINTN Size, > + IN VOID *Buffer > + ) > +{ > + ASSERT (FALSE); > +} > + > + > +/** > + Skip bytes in the firmware configuration item. > + > + Increase the offset of the firmware configuration item without transferring > + bytes between the item and a caller-provided buffer. Subsequent read, write > + or skip operations will commence at the increased offset. > + > + @param[in] Size Number of bytes to skip. > +**/ > +VOID > +EFIAPI > +QemuFwCfgSkipBytes ( > + IN UINTN Size > + ) > +{ > + ASSERT (FALSE); > +} > + > + > +/** > + Reads a UINT8 firmware configuration value > + > + @return Value of Firmware Configuration item read > + > +**/ > +UINT8 > +EFIAPI > +QemuFwCfgRead8 ( > + VOID > + ) > +{ > + ASSERT (FALSE); > + return 0; > +} > + > + > +/** > + Reads a UINT16 firmware configuration value > + > + @return Value of Firmware Configuration item read > + > +**/ > +UINT16 > +EFIAPI > +QemuFwCfgRead16 ( > + VOID > + ) > +{ > + ASSERT (FALSE); > + return 0; > +} > + > + > +/** > + Reads a UINT32 firmware configuration value > + > + @return Value of Firmware Configuration item read > + > +**/ > +UINT32 > +EFIAPI > +QemuFwCfgRead32 ( > + VOID > + ) > +{ > + ASSERT (FALSE); > + return 0; > +} > + > + > +/** > + Reads a UINT64 firmware configuration value > + > + @return Value of Firmware Configuration item read > + > +**/ > +UINT64 > +EFIAPI > +QemuFwCfgRead64 ( > + VOID > + ) > +{ > + ASSERT (FALSE); > + return 0; > +} > + > + > +/** > + Find the configuration item corresponding to the firmware configuration file. > + > + @param[in] Name - Name of file to look up. > + @param[out] Item - Configuration item corresponding to the file, to be passed > + to QemuFwCfgSelectItem (). > + @param[out] Size - Number of bytes in the file. > + > + @return RETURN_SUCCESS If file is found. > + RETURN_NOT_FOUND If file is not found. > + RETURN_UNSUPPORTED If firmware configuration is unavailable. > + > +**/ > +RETURN_STATUS > +EFIAPI > +QemuFwCfgFindFile ( > + IN CONST CHAR8 *Name, > + OUT FIRMWARE_CONFIG_ITEM *Item, > + OUT UINTN *Size > + ) > +{ > + return RETURN_UNSUPPORTED; > +} > + > These parts are fine. Thanks! Laszlo