* [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF @ 2020-11-11 3:10 Rebecca Cran 2020-11-11 19:57 ` Laszlo Ersek 0 siblings, 1 reply; 5+ messages in thread From: Rebecca Cran @ 2020-11-11 3:10 UTC (permalink / raw) To: devel Cc: Rebecca Cran, Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Peter Grehan Fix BhyveX64.dsc and BhyveX64.fdf to follow breaking changes in OVMF. Signed-off-by: Rebecca Cran <rebecca@bsdio.com> --- OvmfPkg/Bhyve/BhyveX64.dsc | 1 + OvmfPkg/Bhyve/BhyveX64.fdf | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc index 16d2233d77..868338b460 100644 --- a/OvmfPkg/Bhyve/BhyveX64.dsc +++ b/OvmfPkg/Bhyve/BhyveX64.dsc @@ -225,6 +225,7 @@ [LibraryClasses.common] BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf + VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf [LibraryClasses.common.SEC] !ifdef $(DEBUG_ON_SERIAL_PORT) diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf index 5d2586ae14..8776aaf7ac 100644 --- a/OvmfPkg/Bhyve/BhyveX64.fdf +++ b/OvmfPkg/Bhyve/BhyveX64.fdf @@ -76,6 +76,12 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid. 0x007000|0x001000 gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize +0x008000|0x001000 +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize + +0x009000|0x002000 +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize + 0x010000|0x010000 gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize -- 2.29.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF 2020-11-11 3:10 [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF Rebecca Cran @ 2020-11-11 19:57 ` Laszlo Ersek 2020-11-11 20:51 ` Lendacky, Thomas 2020-11-12 5:41 ` Rebecca Cran 0 siblings, 2 replies; 5+ messages in thread From: Laszlo Ersek @ 2020-11-11 19:57 UTC (permalink / raw) To: Rebecca Cran Cc: devel, Jordan Justen, Ard Biesheuvel, Peter Grehan, Tom Lendacky +Tom On 11/11/20 04:10, Rebecca Cran wrote: > Fix BhyveX64.dsc and BhyveX64.fdf to follow breaking > changes in OVMF. > > Signed-off-by: Rebecca Cran <rebecca@bsdio.com> > --- > OvmfPkg/Bhyve/BhyveX64.dsc | 1 + > OvmfPkg/Bhyve/BhyveX64.fdf | 6 ++++++ > 2 files changed, 7 insertions(+) Ouch, I'm sorry. > > diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc > index 16d2233d77..868338b460 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.dsc > +++ b/OvmfPkg/Bhyve/BhyveX64.dsc > @@ -225,6 +225,7 @@ > > [LibraryClasses.common] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf > > [LibraryClasses.common.SEC] > !ifdef $(DEBUG_ON_SERIAL_PORT) Yep, makes sense. > diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf > index 5d2586ae14..8776aaf7ac 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.fdf > +++ b/OvmfPkg/Bhyve/BhyveX64.fdf > @@ -76,6 +76,12 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid. > 0x007000|0x001000 > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > +0x008000|0x001000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize > + > +0x009000|0x002000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize > + > 0x010000|0x010000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > Hmm, this, on the other hand, makes me wonder. All four PCDs are [PcdsFixedAtBuild], from "OvmfPkg.dec", so the platform DSC/FDF files *should not* be required to override defaults. .... Ah, wait, you're hitting the exact PCD value checks (%error directives) in "OvmfPkg/ResetVector/ResetVector.nasmb". During the SEV-ES review, I completely lost track of Bhyve consuming "OvmfPkg/ResetVector/ResetVector.inf". Sorry about that. So the following list of commits: (1) 6995a1b79bab OvmfPkg: Create a GHCB page for use during Sec phase (2) 8a2732186a53 OvmfPkg/ResetVector: Add support for a 32-bit SEV check (3) 30937f2f98c4 OvmfPkg: Use the SEV-ES work area for the SEV-ES AP reset vector causes a problem for the Bhyve platform. I don't like the "OvmfPkg/Bhyve/BhyveX64.fdf" hack as presented above, because, while it makes the symptom go away, it causes "BhyveX64.fdf" appear as if it had anything to do with SEV-ES -- which it doesn't. Here's what I suggest: * patch#1: Subject: OvmfPkg/Bhyve: detach ResetVector from before the SEV-ES changes Commit message: Commits 6995a1b79bab, 8a2732186a53 and 30937f2f98c4 modified all four regular files under "OvmfPkg/ResetVector" with SEV-ES dependencies. These are not relevant for Bhyve. Detach the pre-SEV-ES version of ResetVector for Bhyve. Composing the patch: $ git checkout -b bhyve_reset_vector master $ rm -r OvmfPkg/ResetVector/ $ git checkout 6995a1b79bab^ -- OvmfPkg/ResetVector/ $ mv OvmfPkg/ResetVector/ OvmfPkg/Bhyve/ $ git checkout master -- OvmfPkg/ResetVector/ # add your (C) notices to all files under OvmfPkg/Bhyve/ResetVector/ # namely "PageTables64.asm", "ResetVector.inf", "ResetVector.nasmb" # do *not* re-generate the FILE_GUID in the new INF file (this is a # well-known GUID, namely "gEfiFirmwareVolumeTopFileGuid") $ git add OvmfPkg/Bhyve/ResetVector/ $ git commit * patch#2: Subject: OvmfPkg/Bhyve: fix build breakage after SEV-ES changes Commit message: Consume the SEV-ES-independent reset vector restored in the previous patch. Use the Null instance of VmgExitLib. Body: > diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc > index 16d2233d7788..ba79ceef5563 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.dsc > +++ b/OvmfPkg/Bhyve/BhyveX64.dsc > @@ -225,6 +225,7 @@ [LibraryClasses] > > [LibraryClasses.common] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf > > [LibraryClasses.common.SEC] > !ifdef $(DEBUG_ON_SERIAL_PORT) > @@ -571,7 +572,7 @@ [PcdsDynamicHii] > # > ################################################################################ > [Components] > - OvmfPkg/ResetVector/ResetVector.inf > + OvmfPkg/Bhyve/ResetVector/ResetVector.inf > > # > # SEC Phase modules > diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf > index 5d2586ae141a..f4050c4934b7 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.fdf > +++ b/OvmfPkg/Bhyve/BhyveX64.fdf > @@ -117,7 +117,7 @@ [FV.SECFV] > # > INF OvmfPkg/Sec/SecMain.inf > > -INF RuleOverride=RESET_VECTOR OvmfPkg/ResetVector/ResetVector.inf > +INF RuleOverride=RESET_VECTOR OvmfPkg/Bhyve/ResetVector/ResetVector.inf > > ################################################################################ > [FV.PEIFV] Optimally, these changes should have been part of the SEV-ES feature series, but we didn't realize. Sorry about the regression! Thanks Laszlo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF 2020-11-11 19:57 ` Laszlo Ersek @ 2020-11-11 20:51 ` Lendacky, Thomas 2020-11-12 5:41 ` Rebecca Cran 1 sibling, 0 replies; 5+ messages in thread From: Lendacky, Thomas @ 2020-11-11 20:51 UTC (permalink / raw) To: Laszlo Ersek, Rebecca Cran Cc: devel, Jordan Justen, Ard Biesheuvel, Peter Grehan On 11/11/20 1:57 PM, Laszlo Ersek wrote: > +Tom > > On 11/11/20 04:10, Rebecca Cran wrote: >> Fix BhyveX64.dsc and BhyveX64.fdf to follow breaking >> changes in OVMF. >> >> Signed-off-by: Rebecca Cran <rebecca@bsdio.com> >> --- >> OvmfPkg/Bhyve/BhyveX64.dsc | 1 + >> OvmfPkg/Bhyve/BhyveX64.fdf | 6 ++++++ >> 2 files changed, 7 insertions(+) > > Ouch, I'm sorry. I think I missed the Bhyve support being added as I rebased to newer levels of the tree, sorry about that. Thanks, Tom > >> >> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc >> index 16d2233d77..868338b460 100644 >> --- a/OvmfPkg/Bhyve/BhyveX64.dsc >> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc >> @@ -225,6 +225,7 @@ >> >> [LibraryClasses.common] >> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf >> + VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf >> >> [LibraryClasses.common.SEC] >> !ifdef $(DEBUG_ON_SERIAL_PORT) > > Yep, makes sense. > >> diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf >> index 5d2586ae14..8776aaf7ac 100644 >> --- a/OvmfPkg/Bhyve/BhyveX64.fdf >> +++ b/OvmfPkg/Bhyve/BhyveX64.fdf >> @@ -76,6 +76,12 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid. >> 0x007000|0x001000 >> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize >> >> +0x008000|0x001000 >> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize >> + >> +0x009000|0x002000 >> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >> + >> 0x010000|0x010000 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize >> >> > > Hmm, this, on the other hand, makes me wonder. All four PCDs are > [PcdsFixedAtBuild], from "OvmfPkg.dec", so the platform DSC/FDF files > *should not* be required to override defaults. > > .... > > Ah, wait, you're hitting the exact PCD value checks (%error directives) > in "OvmfPkg/ResetVector/ResetVector.nasmb". > > During the SEV-ES review, I completely lost track of Bhyve consuming > "OvmfPkg/ResetVector/ResetVector.inf". Sorry about that. > > So the following list of commits: > > (1) 6995a1b79bab OvmfPkg: Create a GHCB page for use during Sec phase > (2) 8a2732186a53 OvmfPkg/ResetVector: Add support for a 32-bit SEV check > (3) 30937f2f98c4 OvmfPkg: Use the SEV-ES work area for the SEV-ES AP > reset vector > > causes a problem for the Bhyve platform. > > I don't like the "OvmfPkg/Bhyve/BhyveX64.fdf" hack as presented above, > because, while it makes the symptom go away, it causes "BhyveX64.fdf" > appear as if it had anything to do with SEV-ES -- which it doesn't. > > Here's what I suggest: > > > * patch#1: > > Subject: > > OvmfPkg/Bhyve: detach ResetVector from before the SEV-ES changes > > Commit message: > > Commits 6995a1b79bab, 8a2732186a53 and 30937f2f98c4 modified all four > regular files under "OvmfPkg/ResetVector" with SEV-ES dependencies. > These are not relevant for Bhyve. Detach the pre-SEV-ES version of > ResetVector for Bhyve. > > Composing the patch: > > $ git checkout -b bhyve_reset_vector master > $ rm -r OvmfPkg/ResetVector/ > $ git checkout 6995a1b79bab^ -- OvmfPkg/ResetVector/ > $ mv OvmfPkg/ResetVector/ OvmfPkg/Bhyve/ > $ git checkout master -- OvmfPkg/ResetVector/ > > # add your (C) notices to all files under OvmfPkg/Bhyve/ResetVector/ > # namely "PageTables64.asm", "ResetVector.inf", "ResetVector.nasmb" > > # do *not* re-generate the FILE_GUID in the new INF file (this is a > # well-known GUID, namely "gEfiFirmwareVolumeTopFileGuid") > > $ git add OvmfPkg/Bhyve/ResetVector/ > $ git commit > > > * patch#2: > > Subject: > > OvmfPkg/Bhyve: fix build breakage after SEV-ES changes > > Commit message: > > Consume the SEV-ES-independent reset vector restored in the previous > patch. Use the Null instance of VmgExitLib. > > Body: > >> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc >> index 16d2233d7788..ba79ceef5563 100644 >> --- a/OvmfPkg/Bhyve/BhyveX64.dsc >> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc >> @@ -225,6 +225,7 @@ [LibraryClasses] >> >> [LibraryClasses.common] >> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf >> + VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf >> >> [LibraryClasses.common.SEC] >> !ifdef $(DEBUG_ON_SERIAL_PORT) >> @@ -571,7 +572,7 @@ [PcdsDynamicHii] >> # >> ################################################################################ >> [Components] >> - OvmfPkg/ResetVector/ResetVector.inf >> + OvmfPkg/Bhyve/ResetVector/ResetVector.inf >> >> # >> # SEC Phase modules >> diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf >> index 5d2586ae141a..f4050c4934b7 100644 >> --- a/OvmfPkg/Bhyve/BhyveX64.fdf >> +++ b/OvmfPkg/Bhyve/BhyveX64.fdf >> @@ -117,7 +117,7 @@ [FV.SECFV] >> # >> INF OvmfPkg/Sec/SecMain.inf >> >> -INF RuleOverride=RESET_VECTOR OvmfPkg/ResetVector/ResetVector.inf >> +INF RuleOverride=RESET_VECTOR OvmfPkg/Bhyve/ResetVector/ResetVector.inf >> >> ################################################################################ >> [FV.PEIFV] > > Optimally, these changes should have been part of the SEV-ES feature > series, but we didn't realize. Sorry about the regression! > > Thanks > Laszlo > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF 2020-11-11 19:57 ` Laszlo Ersek 2020-11-11 20:51 ` Lendacky, Thomas @ 2020-11-12 5:41 ` Rebecca Cran 2020-11-13 19:39 ` Laszlo Ersek 1 sibling, 1 reply; 5+ messages in thread From: Rebecca Cran @ 2020-11-12 5:41 UTC (permalink / raw) To: Laszlo Ersek Cc: devel, Jordan Justen, Ard Biesheuvel, Peter Grehan, Tom Lendacky On 11/11/20 12:57 PM, Laszlo Ersek wrote: > > Optimally, these changes should have been part of the SEV-ES feature > series, but we didn't realize. Sorry about the regression! I didn't expect people to take on the work of updating Bhyve when making incompatible changes to OvmfPkg, but that would be nice if they could! It's why I have a task to set up a Jenkins CI server, so I can catch regressions earlier. Being a lower priority platform, I suspect it's not something that should go into the existing Azure/Github based CI system. -- Rebecca Cran ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF 2020-11-12 5:41 ` Rebecca Cran @ 2020-11-13 19:39 ` Laszlo Ersek 0 siblings, 0 replies; 5+ messages in thread From: Laszlo Ersek @ 2020-11-13 19:39 UTC (permalink / raw) To: Rebecca Cran Cc: devel, Jordan Justen, Ard Biesheuvel, Peter Grehan, Tom Lendacky On 11/12/20 06:41, Rebecca Cran wrote: > On 11/11/20 12:57 PM, Laszlo Ersek wrote: > >> >> Optimally, these changes should have been part of the SEV-ES feature >> series, but we didn't realize. Sorry about the regression! > > I didn't expect people to take on the work of updating Bhyve when making > incompatible changes to OvmfPkg, but that would be nice if they could! > > It's why I have a task to set up a Jenkins CI server, so I can catch > regressions earlier. Being a lower priority platform, I suspect it's not > something that should go into the existing Azure/Github based CI system. Well, I'm torn. I'd really like the Bhyve platform to be covered with the other OVMF DSC files, in the same CI system. On the other hand, I'm aware that keeping OvmfPkg in the main edk2 repository is not universally welcomed. If Bhyve required additional work (beyond the usual OVMF DSC files) for merging a core package series (such as for MdeModulePkg), *I* personally wouldn't be annoyed in the least ("just do it"), but other contributors -- not as convinced about the benefits of having Bhyve in-tree -- could be. If you don't mind catching Bhyve build issues "slightly after the fact", then I suggest sticking with the status quo. If you have more time to spend, you can still catch regressions early -- watch out for OvmfPkg patches, and whenever something is posted for OvmfPkg, push it through your own CI, and report back *on the list at once* if your build breaks. I guess you could even script this somehow. Thanks! Laszlo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-13 19:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-11 3:10 [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF Rebecca Cran 2020-11-11 19:57 ` Laszlo Ersek 2020-11-11 20:51 ` Lendacky, Thomas 2020-11-12 5:41 ` Rebecca Cran 2020-11-13 19:39 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox