On 05/11/15 20:27, Jordan Justen wrote: > On 2015-05-11 03:21:38, Laszlo Ersek wrote: >> On 05/11/15 11:04, Fan, Jeff wrote: >>> 2. The patch introduced two new files with the different BSD license >>> header format from the other files in the same Package. I do not >>> suggest to use the different BSD license header here. >> >> I don't have any choice in this matter -- only Intel has. The files in >> question are almost verbatim copies (therefore, definitely derivative >> works of): >> >> Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuArchDxe/MtrrSync.h >> Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuArchDxe/MtrrSync.c >> >> and those are covered by Intel's copyright. I can not change the >> copyright terms (3-clause BSDL) set forth by the copyright holder. There >> are four options: >> >> (a) If Intel is willing to relicense these files under the 2-clause BSDL >> (which most files in UefiCpuPkg are under, apparently), then everything >> should be okay. > > This would be nice for consistency of Intel's contributions to EDK II. > Given Intel owns the copyright on the Quark code you referenced, I > would think we ought to be able to retain the licensing consistency in > EDK II. Unforunately, I'm not sure if it will be possible to do. > > I have a few more questions which I think will make incorporating this > code with a separate license even more ugly: > > 1. I note that you didn't add a *second* license header block to the > files you modified. Should you? > * UefiCpuPkg/CpuDxe/CpuDxe.inf > * UefiCpuPkg/CpuDxe/CpuDxe.c > * UefiCpuPkg/UefiCpuPkg.dec > > 2. Does UefiCpuPkg/License.txt need to be updated to have both the > 2-clause and 3-clause BSD licenses listed?
I did think of this after reading Jeff's email -- theoretically, the combined work should be under a license that satisfies the requirements of each part being combined. Since UefiCpuPkg has a top level license that is supposed to cover the entire module (I guess?), this would mean changing that file to the 3-clause BSDL. Honestly, I'm confused. UefiCpuPkg/Contributions.txt lists the 3-clause BSDL as an acceptable license, but it doesn't regulate (and I don't know from myself) how to combine licenses. Strangely, "UefiCpuPkg/Contributions.txt" *itself* lists the non-endorsement clause, the one that is otherwise the difference between the 2-clause BSDL and the 3-clause one, although the majority of the code is under 2-clause. Go figure. Laszlo > > -Jordan > >> (b) Otherwise, you could accept these files with their original 3-clause >> BSDL licenses; it is permitted by "UefiCpuPkg/Contributions.txt". >> >> (c) None of the above works -- in this case we'll have to clone >> UefiCpuPkg/CpuDxe under OvmfPkg, and (maybe?) put all of that CpuDxe >> clone under the 3-clause BSDL. That would be a horrible outcome. >> >> (d) Finally, I can eliminate MTRR handling from my SMM work (see the >> background above), ripping out chunks of code from the CpuMpDxe port and >> the PiSmmCpuDxeSmm port. >> >> (Note, I'm reconstructing CpuMpDxe under a different name in OvmfPkg, >> with only the ACPI_CPU_DATA / S3-related functionality anyway, but >> MtrrTable *does* belong there.) >> >> Removing MTRR handling would be arguably the simplest, but (arguably) >> the least correct thing to do as well. >> >> Thanks >> Laszlo >> >>> Jordan, >>> Any comments on new files' license header? >>> >>> Thanks! >>> Jeff >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:ler...@redhat.com] >>> Sent: Saturday, May 09, 2015 2:52 AM >>> To: edk2-devel@lists.sourceforge.net >>> Cc: Fan, Jeff; Chen Fan; Justen, Jordan L >>> Subject: [PATCH 08/11] UefiCpuPkg: CpuDxe: optionally save MTRR settings to >>> AcpiNVS memory block >>> >>> The >>> >>> Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuArchDxe >>> >>> driver provides the following capability in its implementation of >>> EFI_CPU_ARCH_PROTOCOL: whenever the SetMemoryAttributes() member is used >>> (directly, or indirectly via gDS->SetMemorySpaceAttributes()) to change >>> MTRR settings, the complete set of settings is immediately reflected to an >>> AcpiNVS memory block. >>> >>> (The address of said block is published via a dynamic PCD.) >>> >>> This feature is necessary for supporting SMM-capable S3 resume, hence port >>> it to UefiCpuPkg. In IA32FamilyCpuBasePkg the feature is always built in; >>> here we make it controllable with a feature PCD. >>> >>> Cc: Jeff Fan <jeff....@intel.com> >>> Cc: Chen Fan <chen.fan.f...@cn.fujitsu.com> >>> Cc: Jordan Justen <jordan.l.jus...@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >>> --- >>> UefiCpuPkg/CpuDxe/CpuDxe.inf | 6 + >>> UefiCpuPkg/CpuDxe/MtrrSync.h | 86 ++++++++++++++ >>> UefiCpuPkg/CpuDxe/CpuDxe.c | 17 +++ >>> UefiCpuPkg/CpuDxe/MtrrSync.c | 118 ++++++++++++++++++++ >>> UefiCpuPkg/UefiCpuPkg.dec | 11 ++ >>> 5 files changed, 238 insertions(+) >>> >>> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf >>> index 61bc55a..35e48e1 100644 >>> --- a/UefiCpuPkg/CpuDxe/CpuDxe.inf >>> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf >>> @@ -52,6 +52,8 @@ >>> CpuGdt.h >>> CpuMp.c >>> CpuMp.h >>> + MtrrSync.c >>> + MtrrSync.h >>> >>> [Sources.IA32] >>> Ia32/CpuAsm.asm | MSFT >>> @@ -80,6 +82,10 @@ >>> [Pcd] >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CONSUMES >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CONSUMES >>> + gUefiCpuPkgTokenSpaceGuid.PcdCpuMtrrTableAddress ## >>> SOMETIMES_PRODUCES >>> + >>> +[FeaturePcd] >>> + gUefiCpuPkgTokenSpaceGuid.PcdCpuSyncMtrrToAcpiNvs >>> >>> [Depex] >>> TRUE >>> diff --git a/UefiCpuPkg/CpuDxe/MtrrSync.h b/UefiCpuPkg/CpuDxe/MtrrSync.h >>> new file mode 100644 index 0000000..1c20a32 >>> --- /dev/null >>> +++ b/UefiCpuPkg/CpuDxe/MtrrSync.h >>> @@ -0,0 +1,86 @@ >>> +/** @file >>> + >>> + Include file for MTRR synchronzation. >>> + >>> + Copyright (C) 2015, Red Hat, Inc. >>> + Copyright (c) 2013-2015 Intel Corporation. >>> + >>> + Redistribution and use in source and binary forms, with or without >>> + modification, are permitted provided that the following conditions >>> + are met: >>> + >>> + * Redistributions of source code must retain the above copyright >>> + notice, this list of conditions and the following disclaimer. >>> + * Redistributions in binary form must reproduce the above copyright >>> + notice, this list of conditions and the following disclaimer in the >>> + documentation and/or other materials provided with the distribution. >>> + * Neither the name of Intel Corporation nor the names of its >>> + contributors may be used to endorse or promote products derived from >>> + this software without specific prior written permission. >>> + >>> + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS >>> + "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT >>> + LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR >>> + A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT >>> + OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, >>> + SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT >>> + LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, >>> + DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY >>> + THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >>> + (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE >>> + OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >>> + >>> + Module Name: MtrrSync.h >>> + >>> +**/ >>> + >>> +#ifndef _EFI_MTRR_SYNC_H_ >>> +#define _EFI_MTRR_SYNC_H_ >>> + >>> +#include <Library/MtrrLib.h> >>> + >>> +extern MTRR_SETTINGS *mMtrrTable; >>> + >>> +/** >>> + Initialize memory region for MTRR data. >>> + >>> + This function allocates ACPI NVS memory for MTRR data, and fills the >>> + region with current MTRR data. Each time MTRRs are written, this >>> + memory region will be updated accordingly. >>> + >>> +**/ >>> +VOID >>> +InitializeMtrrData ( >>> + VOID >>> + ); >>> + >>> +/** >>> + Synchronzies up the MTRR values with BSP for calling processor. >>> + >>> + This function synchronzies up the MTRR values with BSP for calling >>> processor. >>> + >>> + @param Buffer Mtrr table address. >>> + >>> +**/ >>> +VOID >>> +EFIAPI >>> +LoadMtrrData ( >>> + VOID *Buffer >>> + ); >>> + >>> +/** >>> + Allocate EfiACPIMemoryNVS below 4G memory address. >>> + >>> + This function allocates EfiACPIMemoryNVS below 4G memory address. >>> + >>> + @param Size Size of memory to allocate. >>> + >>> + @return Allocated address for output. >>> + >>> +**/ >>> +VOID* >>> +AllocateAcpiNvsMemoryBelow4G ( >>> + IN UINTN Size >>> + ); >>> +#endif >>> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c index >>> c9df4e1..3d11807 100644 >>> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c >>> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c >>> @@ -14,6 +14,7 @@ >>> >>> #include "CpuDxe.h" >>> #include "CpuMp.h" >>> +#include "MtrrSync.h" >>> >>> // >>> // Global Variables >>> @@ -405,6 +406,14 @@ CpuSetMemoryAttributes ( >>> CacheType >>> ); >>> >>> + if (!RETURN_ERROR (Status)) { >>> + if (FeaturePcdGet (PcdCpuSyncMtrrToAcpiNvs)) { >>> + // >>> + // Sync saved MTRR settings >>> + // >>> + MtrrGetAllMtrrs (mMtrrTable); >>> + } >>> + } >>> return (EFI_STATUS) Status; >>> } >>> >>> @@ -870,6 +879,14 @@ InitializeCpu ( >>> // >>> ProgramVirtualWireMode (); >>> >>> + if (FeaturePcdGet (PcdCpuSyncMtrrToAcpiNvs)) { >>> + // >>> + // Allocates ACPI NVS memory for MTRR data. >>> + // >>> + InitializeMtrrData (); >>> + MtrrGetAllMtrrs (mMtrrTable); >>> + } >>> + >>> // >>> // Install CPU Architectural Protocol >>> // >>> diff --git a/UefiCpuPkg/CpuDxe/MtrrSync.c b/UefiCpuPkg/CpuDxe/MtrrSync.c >>> new file mode 100644 index 0000000..8538014 >>> --- /dev/null >>> +++ b/UefiCpuPkg/CpuDxe/MtrrSync.c >>> @@ -0,0 +1,118 @@ >>> +/** @file >>> + >>> + Code for MTRR synchronzation. >>> + >>> + Copyright (C) 2015, Red Hat, Inc. >>> + Copyright (c) 2013-2015 Intel Corporation. >>> + >>> + Redistribution and use in source and binary forms, with or without >>> + modification, are permitted provided that the following conditions >>> + are met: >>> + >>> + * Redistributions of source code must retain the above copyright >>> + notice, this list of conditions and the following disclaimer. >>> + * Redistributions in binary form must reproduce the above copyright >>> + notice, this list of conditions and the following disclaimer in the >>> + documentation and/or other materials provided with the distribution. >>> + * Neither the name of Intel Corporation nor the names of its >>> + contributors may be used to endorse or promote products derived from >>> + this software without specific prior written permission. >>> + >>> + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS >>> + "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT >>> + LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR >>> + A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT >>> + OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, >>> + SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT >>> + LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, >>> + DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY >>> + THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >>> + (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE >>> + OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >>> + >>> + Module Name: MtrrSync.c >>> + >>> +**/ >>> + >>> +#include <Library/UefiBootServicesTableLib.h> >>> +#include <Library/DebugLib.h> >>> +#include <Library/BaseMemoryLib.h> >>> + >>> +#include "MtrrSync.h" >>> + >>> +MTRR_SETTINGS *mMtrrTable; >>> + >>> +/** >>> + Initialize memory region for MTRR data. >>> + >>> + This function allocates ACPI NVS memory for MTRR data. >>> + Each time MTRRs are written, this memory region will be updated >>> accordingly. >>> + >>> +**/ >>> +VOID >>> +InitializeMtrrData ( >>> + VOID >>> + ) >>> +{ >>> + // >>> + // Allocate memory for fixed MTRRs, variable MTRRs and MTRR_DEF_TYPE >>> + // >>> + mMtrrTable = AllocateAcpiNvsMemoryBelow4G (sizeof (MTRR_SETTINGS)); >>> + >>> + PcdSet64 (PcdCpuMtrrTableAddress, (UINT64) (UINTN) mMtrrTable); } >>> + >>> +/** >>> + Synchronzies up the MTRR values with BSP for calling processor. >>> + >>> + This function synchronzies up the MTRR values with BSP for calling >>> processor. >>> + >>> + @param Buffer Mtrr table address. >>> + >>> +**/ >>> +VOID >>> +EFIAPI >>> +LoadMtrrData ( >>> + VOID *Buffer >>> + ) >>> +{ >>> + MtrrSetAllMtrrs (Buffer); >>> +} >>> + >>> +/** >>> + Allocate EfiACPIMemoryNVS below 4G memory address. >>> + >>> + This function allocates EfiACPIMemoryNVS below 4G memory address. >>> + >>> + @param Size Size of memory to allocate. >>> + >>> + @return Allocated address for output. >>> + >>> +**/ >>> +VOID* >>> +AllocateAcpiNvsMemoryBelow4G ( >>> + IN UINTN Size >>> + ) >>> +{ >>> + UINTN Pages; >>> + EFI_PHYSICAL_ADDRESS Address; >>> + EFI_STATUS Status; >>> + VOID* Buffer; >>> + >>> + Pages = EFI_SIZE_TO_PAGES (Size); >>> + Address = 0xffffffff; >>> + >>> + Status = gBS->AllocatePages ( >>> + AllocateMaxAddress, >>> + EfiACPIMemoryNVS, >>> + Pages, >>> + &Address >>> + ); >>> + ASSERT_EFI_ERROR (Status); >>> + >>> + Buffer = (VOID *) (UINTN) Address; >>> + ZeroMem (Buffer, Size); >>> + >>> + return Buffer; >>> +} >>> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index >>> 14a0bf2..5297c3b 100644 >>> --- a/UefiCpuPkg/UefiCpuPkg.dec >>> +++ b/UefiCpuPkg/UefiCpuPkg.dec >>> @@ -62,5 +62,16 @@ >>> # @Prompt Configure stack size for Application Processor (AP) >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize|0x8000|UINT32|0x00000003 >>> >>> +[PcdsFeatureFlag] >>> + ## This flag controls whether the EFI_CPU_ARCH_PROTOCOL >>> +implementation >>> + # reflects the updated MTRR settings to a persistent (AcpiNVS) memory >>> block. >>> + # @Prompt Keep MTRR settings reflected to an AcpiNVS memory block >>> + >>> +gUefiCpuPkgTokenSpaceGuid.PcdCpuSyncMtrrToAcpiNvs|FALSE|BOOLEAN|0x00000 >>> +004 >>> + >>> +[PcdsDynamic, PcdsDynamicEx] >>> + ## If PcdCpuSyncMtrrToAcpiNvs is TRUE, the PCD below will be set to >>> +the >>> + # address of the AcpiNVS memory block in question. >>> + >>> +gUefiCpuPkgTokenSpaceGuid.PcdCpuMtrrTableAddress|0x0|UINT64|0x00000005 >>> + >>> [UserExtensions.TianoCore."ExtraFiles"] >>> UefiCpuPkgExtra.uni >>> -- >>> 1.8.3.1 >>> >>> >> ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
