On 11/26/15 17:17, Laszlo Ersek wrote: > On 11/25/15 22:29, Jordan Justen wrote: >> On 2015-11-25 10:06:03, Laszlo Ersek wrote: >>> On 11/25/15 18:46, Jordan Justen wrote: >>>> On 2015-11-03 13:01:17, Laszlo Ersek wrote: >>>> >>>>> + qemu-system-i386 -cpu coreduo,-nx \ >>>> >>>> Sometimes we put '$' before a command prompt. I know there is no >>>> standard prompt, but that is the one that I usually use in examples. >>> >>> Hm, I'm not really consistent on that. When I paste a command line >>> mainly for illustration, I do use $. When I expect people to actually >>> paste the example / snippet into a script, or at the shell prompt >>> directly, then $ just gets in the way of easy line selection. I think in >>> this case I'd prefer to omit the dollar sign. But, you can convince me. >> >> It can get in the way if you just want to copy/paste several commands >> to run right away. I don't think that is the case here. It is just one >> command, and it is going to at least take two copy/pastes regardless. >> I'm not real concerned about this either way. > > Okay, I'll add the dollar sign to the lines where the binaries are entered. > >> >>>> >>>>> + >>>>> +Dependent on the development status of the >>>>> +"UefiCpuPkg/Universal/Acpi/S3Resume2Pei" module, S3 resume may not work >>>>> in >>>>> +OvmfPkg/OvmfPkgX64.dsc builds. In such cases, OvmfPkg/OvmfPkgIa32X64.dsc >>>>> is >>>>> +recommended for running X64 guests. >>>> >>>> Is this paragraph needed? >>> >>> I do think so. People (me included) tend to build the X64 DSC, unless >>> advised otherwie. >> >> Yeah. The order of relavence is definitely X64, then IA32, then >> IA32+X64. >> >>> >>>> I don't think we should have to say that >>>> UefiCpuPkg/S3Resume2Pei might be broken. Is it still broken? >>> >>> I believe so, yes. Jiewen outlined a number of changes required for the >>> PEIM: >>> >>> http://thread.gmane.org/gmane.comp.bios.edk2.devel/3509/focus=3523 >>> >> >> It sounds like we should have the X64 dsc generate a build error with >> SMM_REQUIRED until this is fixed. Maybe that can take the place of >> this comment in the README? > > I disagree with the build error. The X64 SMM_REQUIRE build can work > perfectly fine if you disable S3 in QEMU (that is: dynamically, on the > command line), or if you do enable S3, but never suspend & resume the guest. > > This README patch names the option separately that controls S3 ("-global > ICH9-LPC.disable_s3=[01]"). > > In fact, we (Red Hat) don't support the S3 enabled case (speaking > unofficially -- this has varied over time). It tends to expose bugs in > various components (like guest OS-level drivers). > > The protection that SMM gives (... is supposed to give) to the variable > driver stack is still worthwhile, with S3 disabled. > >> Jiewen, Mike, do you have a plan for solving the X64 PEI >> incompatibility? > > That would be nice, yes, but I don't think it should delay this series > even longer. As far as I understand, no Intel firmware that is released > for physical machines *and* uses SMM builds the module for 64-bit. So > that would be a first. I'm not sure what testing could be done outside > of OVMF, which might delay this even longer. > > Why are you against this paragraph in the README?
Anyway, since you gave your R-b for this patch, I'm going to pick it up, and address all of your remarks, except the one about this paragraph -- I think it is prudent to keep it. It is very easy to remove it when the time comes, and I will very likely notice any changes to S3Resume2Pei in a timely manner (because I always skim fresh commits). Thanks Laszlo > > Thanks > Laszlo > >> >> -Jordan >> >>> This was on Oct 28th. >>> >>> And, whenever I fetch new commits from SVN, I always skim them quickly, >>> before I rebase my local branches on top, so I know what to expect and >>> what patches to edit (possibly) during the rebase. I don't recall any >>> related changes for S3Resume2Pei. >>> >>> But, it's not hard to check at once: >>> >>> $ git log -1 UefiCpuPkg/Universal/Acpi/S3Resume2Pei >>> >>> commit c1fd37cd6bcb98143bd4a44f427735a748058ad8 >>> Author: Hao Wu <hao.a...@intel.com> >>> Date: Mon Jul 13 01:24:24 2015 +0000 >>> >>> UefiCpuPkg S3Resume2Pei: Fix ASSERT in WriteToOsS3PerformanceData >>> >>> That is, July 2015. >>> >>> So yeah, we do need this hint in the README. We can remove it later. >>> >>>> Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com> >>> >>> I'll await your response before picking this up (for just the >>> modifications that I agreed to above). >>> >>> Thanks! >>> Laszlo >>> >>>> >>>>> + >>>>> === Network Support === >>>>> >>>>> OVMF provides a UEFI network stack by default. Its lowest level driver >>>>> is the >>>>> -- >>>>> 1.8.3.1 >>>>> >>>>> _______________________________________________ >>>>> 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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
