On Thu, Nov 04, 2021 at 06:18:10PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > On Mon, Nov 01, 2021 at 10:21:34AM +0000, Dov Murik wrote:
> > > Commit cff03145ed3c ("sev/i386: Introduce sev_add_kernel_loader_hashes
> > > for measured linux boot", 2021-09-30) introduced measured direct boot
> > > with -kernel, using an OVMF-designated hashes table which QEMU fills.
> > >
> > > However, if OVMF doesn't designate such an area, QEMU would completely
> > > abort the VM launch. This breaks launching with -kernel using older
> > > OVMF images which don't publish the SEV_HASH_TABLE_RV_GUID.
> > >
> > > Instead, just warn the user that -kernel was supplied by OVMF doesn't
> > > specify the GUID for the hashes table. The following warning will be
> > > displayed during VM launch:
> > >
> > > qemu-system-x86_64: warning: SEV: kernel specified but OVMF has no
> > > hash table guid
> > >
> > > Signed-off-by: Dov Murik <dovmu...@linux.ibm.com>
> > > Reported-by: Tom Lendacky <thomas.lenda...@amd.com>
> > > ---
> > > target/i386/sev.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > > index eede07f11d..682b8ccf6c 100644
> > > --- a/target/i386/sev.c
> > > +++ b/target/i386/sev.c
> > > @@ -1204,7 +1204,7 @@ bool
> > > sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp)
> > > int aligned_len;
> > >
> > > if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL))
> > > {
> > > - error_setg(errp, "SEV: kernel specified but OVMF has no hash
> > > table guid");
> > > + warn_report("SEV: kernel specified but OVMF has no hash table
> > > guid");
> > > return false;
> >
> > I'm pretty wary of doing this kind of thing.
> >
> > If someone is using QEMU and they required to have the hashes populated
> > for their use case, they now don't get a fatal error if something goes
> > wrong with the process. This is bad as it hides a serious mistake.
> >
> > If someone is using QEMU and they don't require to have the hashes
> > populated and they knowingly use a firmware that doesn't support
> > this, they'll now get a irrelevant warning every time they boot
> > QEMU. This is bad because IME users will file bugs complaining
> > about this bogus warning.
> >
> >
> > If we genuinely need to support both uses cases, then we should have
> > an explicit command line flag to request the desired behaviour.
> >
> > This could be a -machine option to indicate that the hashes must
> > be populated.
> >
> > - unset: try to populate hashes, *silently* ignore missing table
> > in ovmf
> > - set == on: try to populate hashes, report error on missing
> > table in ovmf
> > -set == off: never try to populate hashes, nor look for the
> > table in ovmf
>
> Or as a property on the sev-guest object.
Yep, I thought of that too, and I'm pretty undecided which is "best".
-machine makes sense as 'kernel' and 'initrd' are properties of
the '-machine' and we're doing stuff related to the.
-object sev-guest makes sense as this is behaviour that's (currently)
specific to SEV.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
