Re: [edk2-devel] [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
On Thu, Jul 11, 2019 at 12:24:46PM +0200, Laszlo Ersek wrote: > Every single source file must have both copyright notice(s) and > licensing information. > > - Licensing information without copyright holder(s) makes zero sense -- > the licensing terms are offered / dictated *by* the copyright holders. > > - A copyright notice in itself, without a suitable license (including > the "no license" case) does not provide us with the necessary rights to > carry the source file in the edk2 project. Agree with all of the above. > Regarding "EnableInterrupts.nasm". This looks like a tricky situation. > Under the above link, the file is currently missing a Microsoft (C) > notice (the last commit to modify the file is apparently from Microsoft, > b621971). So that should be addressed in Project Mu first, in my opinion. > > Then, Microsoft should please relicense the file under BSD+Patent, from > pure 2-BSD. If they disagree, then the 2-BSD is still acceptable for edk2. > > Then, the file could be imported into edk2, carrying > - the MS (C) notice, > - the Intel (C) notice (extended to 2019), > - the license -- BSD+Patent, or else 2-BSD --, expressed with an SPDX ID > (not open-coded license text). While I agree it would be better if Microsoft relicensed it as BSD+Patent, BSD+Patent is a strict superset of 2-clause BSD - so there is no point in TianoCore to carry 2-Clause BSD code. We can just add the explicit patent grant on contribution (i.e. change the license header for a BSD+Patent SPDX tag). And we should, because otherwise without the tianocore contribution agreement, we open ourselves up to submarine contributions. The downside to doing that would be if we wanted the code to be able to flow in both directions, which is why it would be good if Microsoft relicensed (and also because then they would be making an explicit statement that they weren't trying to submarine TianoCore). Best Regards, Leif -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43597): https://edk2.groups.io/g/devel/message/43597 Mute This Topic: https://groups.io/mt/32403450/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
On 07/11/19 04:36, Gao, Zhichao wrote: > > >> -Original Message- >> From: Dong, Eric >> Sent: Thursday, July 11, 2019 10:22 AM >> To: Gao, Zhichao ; devel@edk2.groups.io >> Cc: Sean Brogan ; Ni, Ray ; >> Laszlo Ersek ; Gao, Liming ; >> Michael Turner ; Bret Barkelew >> >> Subject: RE: [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol >> >> Hi Zhizhao, >> >> The new add files don't have copyright info, is it ok? Good catch! > I forgot add file comment for Ia32/EnableInterruptsAndSleep.c. Also lose the > BSD+Patent License. I would add them in the future. > For the copyright: > The code is contributed by Microsoft at > https://github.com/Microsoft/mu_basecore/blob/release/201808/MdePkg/Library/BaseLib/X64/EnableInterrupts.nasm#L48 > I move the code from MdePkg to UefiCpuPkg to a single file. It is > inappropriate to add both intel (not contributor) and Microsoft (they didn't > add in the above link). > So I keep the copyright blank. I don't know whether it is ok without > copyright. Every single source file must have both copyright notice(s) and licensing information. - Licensing information without copyright holder(s) makes zero sense -- the licensing terms are offered / dictated *by* the copyright holders. - A copyright notice in itself, without a suitable license (including the "no license" case) does not provide us with the necessary rights to carry the source file in the edk2 project. Regarding "EnableInterrupts.nasm". This looks like a tricky situation. Under the above link, the file is currently missing a Microsoft (C) notice (the last commit to modify the file is apparently from Microsoft, b621971). So that should be addressed in Project Mu first, in my opinion. Then, Microsoft should please relicense the file under BSD+Patent, from pure 2-BSD. If they disagree, then the 2-BSD is still acceptable for edk2. Then, the file could be imported into edk2, carrying - the MS (C) notice, - the Intel (C) notice (extended to 2019), - the license -- BSD+Patent, or else 2-BSD --, expressed with an SPDX ID (not open-coded license text). Note: I'm not a lawyer; the above is just my opinion. CC'ing the other stewards. Thanks, Laszlo > > Hi Liming, > > What's your opinion? > > Thanks, > Zhichao > >> >> Thanks, >> Eric >> >>> -Original Message- >>> From: Gao, Zhichao >>> Sent: Tuesday, July 9, 2019 4:40 PM >>> To: devel@edk2.groups.io >>> Cc: Sean Brogan ; Dong, Eric >>> ; Ni, Ray ; Laszlo Ersek >>> ; Gao, Liming ; Michael >>> Turner ; Bret Barkelew >>> >>> Subject: [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol >>> >>> From: Sean Brogan >>> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400 >>> >>> Implement Cp2 protocol: it has one interface to enable the interrupt >>> and put cpu to sleep and wait for an interrupt. >>> >>> Cc: Eric Dong >>> Cc: Ray Ni >>> Cc: Laszlo Ersek >>> Cc: Liming Gao >>> Cc: Sean Brogan >>> Cc: Michael Turner >>> Cc: Bret Barkelew >>> Signed-off-by: Zhichao Gao >>> --- >>> UefiCpuPkg/CpuDxe/CpuDxe.c| 38 +++ >>> UefiCpuPkg/CpuDxe/CpuDxe.h| 25 >>> UefiCpuPkg/CpuDxe/CpuDxe.inf | 3 ++ >>> .../CpuDxe/Ia32/EnableInterruptsAndSleep.c| 24 >>> .../CpuDxe/X64/EnableInterruptsAndSleep.nasm | 31 +++ >>> 5 files changed, 121 insertions(+) >>> create mode 100644 >> UefiCpuPkg/CpuDxe/Ia32/EnableInterruptsAndSleep.c >>> create mode 100644 >>> UefiCpuPkg/CpuDxe/X64/EnableInterruptsAndSleep.nasm >>> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43596): https://edk2.groups.io/g/devel/message/43596 Mute This Topic: https://groups.io/mt/32403450/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
> -Original Message- > From: Dong, Eric > Sent: Thursday, July 11, 2019 10:22 AM > To: Gao, Zhichao ; devel@edk2.groups.io > Cc: Sean Brogan ; Ni, Ray ; > Laszlo Ersek ; Gao, Liming ; > Michael Turner ; Bret Barkelew > > Subject: RE: [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol > > Hi Zhizhao, > > The new add files don't have copyright info, is it ok? I forgot add file comment for Ia32/EnableInterruptsAndSleep.c. Also lose the BSD+Patent License. I would add them in the future. For the copyright: The code is contributed by Microsoft at https://github.com/Microsoft/mu_basecore/blob/release/201808/MdePkg/Library/BaseLib/X64/EnableInterrupts.nasm#L48 I move the code from MdePkg to UefiCpuPkg to a single file. It is inappropriate to add both intel (not contributor) and Microsoft (they didn't add in the above link). So I keep the copyright blank. I don't know whether it is ok without copyright. Hi Liming, What's your opinion? Thanks, Zhichao > > Thanks, > Eric > > > -Original Message- > > From: Gao, Zhichao > > Sent: Tuesday, July 9, 2019 4:40 PM > > To: devel@edk2.groups.io > > Cc: Sean Brogan ; Dong, Eric > > ; Ni, Ray ; Laszlo Ersek > > ; Gao, Liming ; Michael > > Turner ; Bret Barkelew > > > > Subject: [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol > > > > From: Sean Brogan > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400 > > > > Implement Cp2 protocol: it has one interface to enable the interrupt > > and put cpu to sleep and wait for an interrupt. > > > > Cc: Eric Dong > > Cc: Ray Ni > > Cc: Laszlo Ersek > > Cc: Liming Gao > > Cc: Sean Brogan > > Cc: Michael Turner > > Cc: Bret Barkelew > > Signed-off-by: Zhichao Gao > > --- > > UefiCpuPkg/CpuDxe/CpuDxe.c| 38 +++ > > UefiCpuPkg/CpuDxe/CpuDxe.h| 25 > > UefiCpuPkg/CpuDxe/CpuDxe.inf | 3 ++ > > .../CpuDxe/Ia32/EnableInterruptsAndSleep.c| 24 > > .../CpuDxe/X64/EnableInterruptsAndSleep.nasm | 31 +++ > > 5 files changed, 121 insertions(+) > > create mode 100644 > UefiCpuPkg/CpuDxe/Ia32/EnableInterruptsAndSleep.c > > create mode 100644 > > UefiCpuPkg/CpuDxe/X64/EnableInterruptsAndSleep.nasm > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43549): https://edk2.groups.io/g/devel/message/43549 Mute This Topic: https://groups.io/mt/32403450/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
Hi Zhizhao,
The new add files don't have copyright info, is it ok?
Thanks,
Eric
> -Original Message-
> From: Gao, Zhichao
> Sent: Tuesday, July 9, 2019 4:40 PM
> To: devel@edk2.groups.io
> Cc: Sean Brogan ; Dong, Eric
> ; Ni, Ray ; Laszlo Ersek
> ; Gao, Liming ; Michael Turner
> ; Bret Barkelew
>
> Subject: [PATCH V2 2/4] UefiCpuPkg/CpuDxe: Implement Cpu2 protocol
>
> From: Sean Brogan
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1400
>
> Implement Cp2 protocol: it has one interface to enable the interrupt and put
> cpu to sleep and wait for an interrupt.
>
> Cc: Eric Dong
> Cc: Ray Ni
> Cc: Laszlo Ersek
> Cc: Liming Gao
> Cc: Sean Brogan
> Cc: Michael Turner
> Cc: Bret Barkelew
> Signed-off-by: Zhichao Gao
> ---
> UefiCpuPkg/CpuDxe/CpuDxe.c| 38 +++
> UefiCpuPkg/CpuDxe/CpuDxe.h| 25
> UefiCpuPkg/CpuDxe/CpuDxe.inf | 3 ++
> .../CpuDxe/Ia32/EnableInterruptsAndSleep.c| 24
> .../CpuDxe/X64/EnableInterruptsAndSleep.nasm | 31 +++
> 5 files changed, 121 insertions(+)
> create mode 100644 UefiCpuPkg/CpuDxe/Ia32/EnableInterruptsAndSleep.c
> create mode 100644
> UefiCpuPkg/CpuDxe/X64/EnableInterruptsAndSleep.nasm
>
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
> index 7d7270e10b..2eeffcf426 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
> @@ -18,6 +18,7 @@
> //
> BOOLEAN InterruptState = FALSE;
> EFI_HANDLEmCpuHandle = NULL;
> +EFI_HANDLEmCpu2Handle = NULL;
> BOOLEAN mIsFlushingGCD;
> BOOLEAN mIsAllocatingPageTable = FALSE;
> UINT64mValidMtrrAddressMask;
> @@ -96,6 +97,10 @@ EFI_CPU_ARCH_PROTOCOL gCpu = {
>4 // DmaBufferAlignment
> };
>
> +EDKII_CPU2_PROTOCOL gCpu2 = {
> + CpuEnableAndWaitForInterrupt
> +};
> +
> //
> // CPU Arch Protocol Functions
> //
> @@ -499,6 +504,28 @@ CpuSetMemoryAttributes (
>return AssignMemoryPageAttributes (NULL, BaseAddress, Length,
> MemoryAttributes, NULL); }
>
> +//
> +// CPU2 Protocol Functions
> +//
> +/**
> + This function enables CPU interrupts and then waits for an interrupt to
> arrive.
> +
> + @param This The EFI_CPU2_PROTOCOL instance.
> +
> + @retval EFI_SUCCESS Interrupts are enabled on the processor.
> + @retval EFI_DEVICE_ERROR Interrupts could not be enabled on the
> processor.
> +
> +**/
> +EFI_STATUS
> +CpuEnableAndWaitForInterrupt (
> + IN EDKII_CPU2_PROTOCOL *This
> + )
> +{
> + EnableInterruptsAndSleep ();
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
>Initializes the valid bits mask and valid address mask for MTRRs.
>
> @@ -1211,6 +1238,17 @@ InitializeCpu (
>);
>ASSERT_EFI_ERROR (Status);
>
> + //
> + // Install CPU2 Protocol
> + //
> + Status = gBS->InstallMultipleProtocolInterfaces (
> + ,
> + ,
> + ,
> + NULL
> + );
> + ASSERT_EFI_ERROR (Status);
> +
>InitializeMpSupport ();
>
>return Status;
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
> index b029be430b..c1398830ba 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.h
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
> @@ -12,6 +12,7 @@
> #include
>
> #include
> +#include
> #include
> #include
>
> @@ -305,6 +306,30 @@ PageFaultExceptionHandler (
>IN EFI_SYSTEM_CONTEXT SystemContext
>);
>
> +/**
> + This function enables CPU interrupts and then waits for an interrupt to
> arrive.
> +
> + @param This The EFI_CPU2_PROTOCOL instance.
> +
> + @retval EFI_SUCCESS Interrupts are enabled on the processor.
> + @retval EFI_DEVICE_ERROR Interrupts could not be enabled on the
> processor.
> +
> +**/
> +EFI_STATUS
> +CpuEnableAndWaitForInterrupt (
> + IN EDKII_CPU2_PROTOCOL *This
> + );
> +
> +/**
> + Enables CPU interrupts and then waits for an interrupt to arrive.
> +
> +**/
> +VOID
> +EFIAPI
> +EnableInterruptsAndSleep (
> + VOID
> + );
> +
> extern BOOLEAN mIsAllocatingPageTable;
> extern UINTN mNumberOfProcessors;
>
> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf
> b/UefiCpuPkg/CpuDxe/CpuDxe.inf index 57381dbc85..334ddb142f 100644
> --- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
> @@ -54,14 +54,17 @@
>
> [Sources.IA32]
>Ia32/CpuAsm.nasm
> + Ia32/EnableInterruptsAndSleep.c
>
> [Sources.X64]
>X64/CpuAsm.nasm
> + X64/EnableInterruptsAndSleep.nasm
>
> [Protocols]
>gEfiCpuArchProtocolGuid ## PRODUCES
>gEfiMpServiceProtocolGuid ## PRODUCES
>gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
> + gEdkiiCpu2ProtocolGuid## PRODUCES
>
> [Guids]
>gIdleLoopEventGuid
