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? -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
