On 05/09/16 17:17, Chang, Abner (HPS SW/FW Technologist) wrote: > It's fine. I will have a copy for RISC-V. Simple!
Yes, that is a perfectly valid strategy. Thanks Laszlo > Thanks > > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Monday, May 09, 2016 11:03 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/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
