Laszlo, Thanks for the feedback.
1) I have updated the author using --author='Laszlo Ersek <ler...@redhat.com>' 2) I changed the order of the commits so the MP race condition is fixed before your patch to sync MTRRs to all APs. Best regards, Mike >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Laszlo Ersek >Sent: Monday, October 19, 2015 6:43 AM >To: Kinney, Michael D; edk2-de...@ml01.01.org >Cc: edk2-devel >Subject: Re: [edk2] [PATCH v4 03/19] UefiCpuPkg: CpuDxe: broadcast MTRR >changes to APs > >On 10/19/15 09:44, Michael Kinney wrote: >> From: edk2-devel <edk2-devel-boun...@lists.01.org> > >With SVN it won't matter much ultimately, but considering git per se, I >think in your git tree my authorship was lost somehow. (The >Signed-off-by below is correct though.) This can be fixed by: >- rebasing the series >- selecting "edit" for this patch >- committing it without any changes, but with different authorship: > git commit --amend --author='Laszlo Ersek <ler...@redhat.com>' > >Not overly important, but I think it's useful to know about git commit >--author. > >Another (not super-important) comment: in cases like this, a patch >series usually applies the fix for the dormant bug first (i.e., patch v4 >04/19), and then introduces the change that would expose the bug (i.e., >this patch). The idea being, since the bug is known, there should be no >point in history where the bug is exposed -- it could be triggered by an >unrelated bisection, for example. > >Again, not too important, but it's so great that you've been using git >for this work that I can't hold back some unsolicited git tips. :) > >Thanks! >Laszlo > >> >> The Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuArchDxe >> driver applies any MTRR changes to APs, if the >> EFI_MP_SERVICES_PROTOCOL is available. We should do the same. >> >> Additionally, the broadcast should occur at MP startup as well, >> not only when MTRR settings are changed. The inspiration is >> taken from >> >> Quark_EDKII_v1.1.0/IA32FamilyCpuBasePkg/CpuMpDxe/ >> >> (see the EarlyMpInit() function and its call sites in >> "ProcessorConfig.c"). >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> Reviewed-by: Jeff Fan <jeff....@intel.com> >> Reviewed-by: Michael Kinney <michael.d.kin...@intel.com> >> --- >> UefiCpuPkg/CpuDxe/CpuDxe.c | 26 ++++++++++++++++++++++++++ >> UefiCpuPkg/CpuDxe/CpuMp.c | 34 +++++++++++++++++++++++++++++++++- >> UefiCpuPkg/CpuDxe/CpuMp.h | 13 +++++++++++++ >> 3 files changed, 72 insertions(+), 1 deletion(-) >> >> diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c >> index c9df4e1..daf97bd 100644 >> --- a/UefiCpuPkg/CpuDxe/CpuDxe.c >> +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c >> @@ -350,6 +350,9 @@ CpuSetMemoryAttributes ( >> { >> RETURN_STATUS Status; >> MTRR_MEMORY_CACHE_TYPE CacheType; >> + EFI_STATUS MpStatus; >> + EFI_MP_SERVICES_PROTOCOL *MpService; >> + MTRR_SETTINGS MtrrSettings; >> >> if (!IsMtrrSupported ()) { >> return EFI_UNSUPPORTED; >> @@ -405,6 +408,29 @@ CpuSetMemoryAttributes ( >> CacheType >> ); >> >> + if (!RETURN_ERROR (Status)) { >> + MpStatus = gBS->LocateProtocol ( >> + &gEfiMpServiceProtocolGuid, >> + NULL, >> + (VOID **)&MpService >> + ); >> + // >> + // Synchronize the update with all APs >> + // >> + if (!EFI_ERROR (MpStatus)) { >> + MtrrGetAllMtrrs (&MtrrSettings); >> + MpStatus = MpService->StartupAllAPs ( >> + MpService, // This >> + SetMtrrsFromBuffer, // Procedure >> + TRUE, // SingleThread >> + NULL, // WaitEvent >> + 0, // TimeoutInMicrosecsond >> + &MtrrSettings, // ProcedureArgument >> + NULL // FailedCpuList >> + ); >> + ASSERT (MpStatus == EFI_SUCCESS || MpStatus == EFI_NOT_STARTED); >> + } >> + } >> return (EFI_STATUS) Status; >> } >> >> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c >> index da3686e..fbe43f5 100644 >> --- a/UefiCpuPkg/CpuDxe/CpuMp.c >> +++ b/UefiCpuPkg/CpuDxe/CpuMp.c >> @@ -1626,6 +1626,22 @@ ExitBootServicesCallback ( >> } >> >> /** >> + A minimal wrapper function that allows MtrrSetAllMtrrs() to be passed to >> + EFI_MP_SERVICES_PROTOCOL.StartupAllAPs() as Procedure. >> + >> + @param[in] Buffer Pointer to an MTRR_SETTINGS object, to be passed to >> + MtrrSetAllMtrrs(). >> +**/ >> +VOID >> +EFIAPI >> +SetMtrrsFromBuffer ( >> + IN VOID *Buffer >> + ) >> +{ >> + MtrrSetAllMtrrs (Buffer); >> +} >> + >> +/** >> Initialize Multi-processor support. >> >> **/ >> @@ -1634,7 +1650,8 @@ InitializeMpSupport ( >> VOID >> ) >> { >> - EFI_STATUS Status; >> + EFI_STATUS Status; >> + MTRR_SETTINGS MtrrSettings; >> >> gMaxLogicalProcessorNumber = (UINTN) PcdGet32 >(PcdCpuMaxLogicalProcessorNumber); >> if (gMaxLogicalProcessorNumber < 1) { >> @@ -1690,6 +1707,21 @@ InitializeMpSupport ( >> // >> CollectBistDataFromHob (); >> >> + // >> + // Synchronize MTRR settings to APs. >> + // >> + MtrrGetAllMtrrs (&MtrrSettings); >> + Status = mMpServicesTemplate.StartupAllAPs ( >> + &mMpServicesTemplate, // This >> + SetMtrrsFromBuffer, // Procedure >> + TRUE, // SingleThread >> + NULL, // WaitEvent >> + 0, // >> TimeoutInMicrosecsond >> + &MtrrSettings, // ProcedureArgument >> + NULL // FailedCpuList >> + ); >> + ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_STARTED); >> + >> Status = gBS->InstallMultipleProtocolInterfaces ( >> &mMpServiceHandle, >> &gEfiMpServiceProtocolGuid, &mMpServicesTemplate, >> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.h b/UefiCpuPkg/CpuDxe/CpuMp.h >> index d2866e4..503f3ae 100644 >> --- a/UefiCpuPkg/CpuDxe/CpuMp.h >> +++ b/UefiCpuPkg/CpuDxe/CpuMp.h >> @@ -643,5 +643,18 @@ ResetApStackless ( >> IN UINT32 ProcessorId >> ); >> >> +/** >> + A minimal wrapper function that allows MtrrSetAllMtrrs() to be passed to >> + EFI_MP_SERVICES_PROTOCOL.StartupAllAPs() as Procedure. >> + >> + @param[in] Buffer Pointer to an MTRR_SETTINGS object, to be passed to >> + MtrrSetAllMtrrs(). >> +**/ >> +VOID >> +EFIAPI >> +SetMtrrsFromBuffer ( >> + IN VOID *Buffer >> + ); >> + >> #endif // _CPU_MP_H_ >> >> > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
