On 05/09/16 16:27, Chang, Abner (HPS SW/FW Technologist) wrote: > Thanks! Laszlo. > The subject is truncated, not sure why. Will resend the patches. > QemuFwCfgLib is good to stay under OVMF. This is an common interface to talk > to QEMU. We just don't use QEMU FwCfg on QEMU RISC-V port, that's why it left > as empty.
In other words, QEMU offers a common firmware config interface that - ARM and AARCH64 QEMU guests use: ArmVirtPkg/Library/QemuFwCfgLib - Ia32 and X64 guest use: OvmfPkg/Library/QemuFwCfgLib - and RISC-V virtual machines do *not* use. If you want a library instance that provides stubs for interfaces of a library class, that is fully supported by edk2. The way it is supported is to introduce a BaseWhateverLibNull library instance, and to resolve the library class to this library instance in the platform DSC file that builds the firmware for this platfrom. > I will use MMIO in RISC-V IoRead(write) function instead of having another > copy of QemuFwCfgLib under RiscVVirtPkg. FwCfg may be used one day for RISC-V. Sorry, this is completely wrong. Writing OvmfPkg code for a device that is neither currently available in upstream QEMU (or Xen), nor being worked on as a feature, is a recipe for disaster. Laszlo > Thanks > Abner > > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Monday, May 09, 2016 5:10 PM > To: Chang, Abner (HPS SW/FW Technologist) <[email protected]>; > [email protected] > Cc: [email protected]; AbnerChang <[email protected]> > Subject: Re: [edk2] [PATCH 2/2] OvmfPkg/QemuFwCfgLib: > > On 05/08/16 06:40, Abner Chang wrote: >> From: AbnerChang <[email protected]> >> >> QEMU RISC-V I/O lib instance. Add RISC-V I/O lib instance for QEMU FW config >> lib. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Abner Chang<[email protected]> >> >> --- >> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf | 7 ++- >> OvmfPkg/Library/QemuFwCfgLib/RiscV64/IoLibEx.c | 80 >> ++++++++++++++++++++++++++ >> 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 >> OvmfPkg/Library/QemuFwCfgLib/RiscV64/IoLibEx.c > > The commit message is both malformed and incorrect. Malformed because the > subject line seems to be truncated. And incorrect because the patch does not > add a new library instance, it modifies one of the library instances. > > The patch seems to plug empty stub implementations into IoWriteFifo8() and > IoReadFifo8(). Given that is the workhorse function that underlies all other > read functions in the QemuFwCfgLib instances that live inside this directory, > the patch seems to aim at allowing QemuFwCfgLib to build and run on RISC-V, > without actually doing anything. > > If that's the case, then instead of this patch, a Null library instance > should be implemented for QemuFwCfgLib, under the > > RiscVVirtPkg/Library/BaseQemuFwCfgLibNull > > directory. > > Thanks > Laszlo > >> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf >> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf >> index a95e1e7..eaefd17 100644 >> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf >> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf >> @@ -2,6 +2,7 @@ >> # >> # Stateful, implicitly initialized fw_cfg library. >> # >> +# Copyright (c) 2016, Hewlett Packard Enterprise Development LP. All >> +rights reserved.<BR> >> # Copyright (C) 2013, Red Hat, Inc. >> # Copyright (c) 2008 - 2012, Intel Corporation. All rights >> reserved.<BR> # @@ -28,7 +29,7 @@ # # The following information is >> for reference only and not required by the build tools. >> # >> -# VALID_ARCHITECTURES = IA32 X64 >> +# VALID_ARCHITECTURES = IA32 X64 RISCV64 >> # >> >> [Sources] >> @@ -40,6 +41,10 @@ >> >> [Sources.X64] >> X64/IoLibExAsm.nasm >> + >> +[Sources.RISCV64] >> + RiscV64/IoLibEx.c >> + >> >> [Packages] >> MdePkg/MdePkg.dec >> diff --git a/OvmfPkg/Library/QemuFwCfgLib/RiscV64/IoLibEx.c >> b/OvmfPkg/Library/QemuFwCfgLib/RiscV64/IoLibEx.c >> new file mode 100644 >> index 0000000..8c09bba >> --- /dev/null >> +++ b/OvmfPkg/Library/QemuFwCfgLib/RiscV64/IoLibEx.c >> @@ -0,0 +1,80 @@ >> +/** @file >> + RISC-V specific functionality for I/O read/write. >> + >> + Copyright (c) 2016, Hewlett Packard Enterprise Development LP. All >> + rights reserved.<BR> >> + >> + 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 http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >> +BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >> IMPLIED. >> +**/ >> + >> +#include "Uefi.h" >> +#include <Library/BaseLib.h> >> +#include <Library/BaseMemoryLib.h> >> +#include <Library/DebugLib.h> >> +#include <Library/IoLib.h> >> +#include <Library/QemuFwCfgLib.h> >> +#include <Library/MemoryAllocationLib.h> #include >> +<Library/UefiBootServicesTableLib.h> >> + >> +/** >> + Reads an 8-bit I/O port fifo into a block of memory. >> + >> + Reads the 8-bit I/O fifo port specified by Port. >> + >> + The port is read Count times, and the read data is stored in the >> + provided Buffer. >> + >> + This function must guarantee that all I/O read and write operations >> + are serialized. >> + >> + If 8-bit I/O port operations are not supported, then ASSERT(). >> + >> + @param Port The I/O port to read. >> + @param Count The number of times to read I/O port. >> + @param Buffer The buffer to store the read data into. >> + >> +**/ >> +VOID >> +EFIAPI >> +IoReadFifo8 ( >> + IN UINTN Port, >> + IN UINTN Count, >> + OUT VOID *Buffer >> + ) >> +{ >> +} >> + >> +/** >> + Writes an 8-bit I/O port fifo from a block of memory. >> + >> + Writes the 8-bit I/O fifo port specified by Port. >> + >> + The port is written Count times, and the data are obtained from >> + the provided Buffer. >> + >> + This function must guarantee that all I/O read and write operations >> + are serialized. >> + >> + If 8-bit I/O port operations are not supported, then ASSERT(). >> + >> + @param Port The I/O port to read. >> + @param Count The number of times to read I/O port. >> + @param Buffer The buffer to store the read data into. >> + >> +**/ >> +VOID >> +EFIAPI >> +IoWriteFifo8 ( >> + IN UINTN Port, >> + IN UINTN Count, >> + OUT VOID *Buffer >> + ) >> +{ >> +} >> + >> + >> > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
