On 2015-10-25 19:53:23, Paolo Bonzini wrote:
> On 23/10/2015 17:29, Laszlo Ersek wrote:
> > I plan to drop this patch, dependent on testing, and on how a new QEMU
> > patch I've written will be received on qemu-devel.
> 
> I'm not sure why it can't be fixed within the firmware.  Your patch
> to QEMU to use current_cpu obviously makes sense (that's why it
> has been merged already :)), but otherwise the firmware should
> adjust to the hardware, not the other way round.

In general I agree with that. But QEMU, at least loosely, attempts to
emulate actual hardware. A discrepancy has been noted in that
emulation, so it seems worth discussing whether it could be addressed.

Yes this would change the QEMU emulated "hardware" behavior, but it
still seems like something QEMU could change in pc-q35-2.5 and newer,
while maintaining the older behavior for <= 2.4.

Laszlo's patch did change the hardware behavior, but I guess since it
doesn't impact seabios there is no need to maintain the old behavior
on older q35 machines?

-Jordan

> Perhaps we can use a new sync mode (a new PCD would be neater, but
> you'd have to pass it to BSPHandler and APHandler) to send the
> IPI from SMM.  A simple implementation of the former would be this:
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index dd333a1..1be1a4d 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -377,7 +377,7 @@
>  !endif
>  
>  !if $(SMM_REQUIRE) == TRUE
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x02
>    gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|1
>  !endif
>  
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 9f1ed34..430f1f4 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -383,7 +383,7 @@
>  
>  [PcdsFixedAtBuild.X64]
>  !if $(SMM_REQUIRE) == TRUE
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x02
>    gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|2
>  !endif
>  
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index c0cc92b..b052806 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -382,7 +382,7 @@
>  !endif
>  
>  !if $(SMM_REQUIRE) == TRUE
> -  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x02
>    gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes|2
>  !endif
>  
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index b0191cb..7b20e27 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -191,6 +191,29 @@ AllCpusInSmmWithExceptions (
>  }
>  
>  
> +VOID
> +SmmSendSmiToAPs (
> +  VOID
> +  )
> +{
> +  UINTN                             Index;
> +
> +  ASSERT (mSmmMpSyncData->Counter <= mNumberOfCpus);
> +
> +  if (mSmmMpSyncData->Counter < mNumberOfCpus) {
> +    //
> +    // Send SMI IPIs to bring outside processors in
> +    //
> +    for (Index = mMaxNumberOfCpus; Index-- > 0;) {
> +      if (!mSmmMpSyncData->CpuData[Index].Present && 
> gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId != INVALID_APIC_ID) {
> +        SendSmiIpi 
> ((UINT32)gSmmCpuPrivate->ProcessorInfo[Index].ProcessorId);
> +      }
> +    }
> +  }
> +
> +  return;
> +}
> +
>  /**
>    Given timeout constraint, wait for all APs to arrive, and insure when this 
> function returns, no AP will execute normal mode code before
>    entering SMM, except SMI disabled APs.
> @@ -344,6 +367,16 @@ BSPHandler (
>    //
>    gSmmCpuPrivate->SmmCoreEntryContext.CurrentlyExecutingCpu = CpuIndex;
>    
> +  //
> +  // Manual AP Sync Mode: SmmControl2DxeTrigger only triggers an SMI
> +  // on the processor that executed it, call all other APs.  Otherwise
> +  // this is the same as Relaxed mode.
> +  //
> +  if (SyncMode == SmmCpuSyncModeManualAp) {
> +    SmmSendSmiToAPs();
> +  }
> +
>    //
>    // If Traditional Sync Mode or need to configure MTRRs: gather all 
> available APs.
>    //
> @@ -450,12 +482,11 @@ BSPHandler (
>    ClearSmi();
>  
>    //
> -  // If Relaxed-AP Sync Mode: gather all available APs after BSP SMM 
> handlers are done, and
> -  // make those APs to exit SMI synchronously. APs which arrive later will 
> be excluded and 
> +  // If Relaxed-AP or Manual-AP Sync Mode: gather all available APs after 
> BSP SMM handlers
> +  // are done, and make those APs to exit SMI synchronously. APs which 
> arrive later will be
>    // will run through freely.
>    //
>    if (SyncMode != SmmCpuSyncModeTradition && 
> !SmmCpuFeaturesNeedConfigureMtrrs()) {
> -
>      //
>      // Lock the counter down and retrieve the number of APs
>      //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 71f9b1b..a631811 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -297,6 +297,7 @@ typedef struct {
>  typedef enum {
>    SmmCpuSyncModeTradition,
>    SmmCpuSyncModeRelaxedAp,
> +  SmmCpuSyncModeManualAp,
>    SmmCpuSyncModeMax
>  } SMM_CPU_SYNC_MODE;
>  
> 
> Paolo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to