All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Lendacky <thomas.lendacky@amd.com>
To: Dov Murik <dovmurik@linux.ibm.com>, qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Ashish Kalra" <ashish.kalra@amd.com>,
	"Brijesh Singh" <brijesh.singh@amd.com>,
	"Tobin Feldman-Fitzthum" <tobin@linux.ibm.com>,
	"James Bottomley" <jejb@linux.ibm.com>
Subject: Re: [PATCH 1/3] sev/i386: Allow launching with -kernel if no OVMF hashes table found
Date: Mon, 1 Nov 2021 09:25:25 -0500	[thread overview]
Message-ID: <24823e7a-b4e6-150c-df41-a373850301a7@amd.com> (raw)
In-Reply-To: <20211101102136.1706421-2-dovmurik@linux.ibm.com>

On 11/1/21 5:21 AM, 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 <dovmurik@linux.ibm.com>
> Reported-by: Tom Lendacky <thomas.lendacky@amd.com>

Just a few minor comments/questions below, otherwise:

Acked-by: Tom Lendacky <thomas.lendacky@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");

Not sure if warn or info would be better, either way works for me, though, 
since this allows the guest to boot now.

Unrelated to this change, but do we explicitly want to say OVMF? Can't 
another BIOS/UEFI implementation have this support?

and should guid be GUID?

Thanks,
Tom

>           return false;
>       }
>       area = (SevHashTableDescriptor *)data;
> 


  reply	other threads:[~2021-11-01 14:32 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-01 10:21 [PATCH 0/3] SEV: fixes for -kernel launch with incompatible OVMF Dov Murik
2021-11-01 10:21 ` [PATCH 1/3] sev/i386: Allow launching with -kernel if no OVMF hashes table found Dov Murik
2021-11-01 14:25   ` Tom Lendacky [this message]
2021-11-01 17:56     ` Dov Murik
2021-11-03 16:02   ` Daniel P. Berrangé
2021-11-04 18:18     ` Dr. David Alan Gilbert
2021-11-04 18:22       ` Daniel P. Berrangé
2021-11-05  7:41         ` Dov Murik
2021-11-01 10:21 ` [PATCH 2/3] sev/i386: Warn if using -kernel with invalid OVMF hashes table area Dov Murik
2021-11-02 12:36   ` Dr. David Alan Gilbert
2021-11-02 12:56     ` Dov Murik
2021-11-02 18:38       ` Dr. David Alan Gilbert
2021-11-02 19:00         ` Philippe Mathieu-Daudé
2021-11-03 16:07   ` Daniel P. Berrangé
2021-11-05  7:52     ` Dov Murik
2021-11-01 10:21 ` [PATCH 3/3] sev/i386: Perform padding calculations at compile-time Dov Murik
2021-11-02 11:36   ` Dr. David Alan Gilbert
2021-11-02 11:50     ` Dov Murik
2021-11-03 14:49   ` Philippe Mathieu-Daudé
2021-11-02 10:52 ` [PATCH 0/3] SEV: fixes for -kernel launch with incompatible OVMF Brijesh Singh
2021-11-02 13:22   ` Dov Murik
2021-11-02 14:48     ` Brijesh Singh
2021-11-03 14:08       ` Dr. David Alan Gilbert
2021-11-03 15:44         ` Brijesh Singh
2021-11-05  7:38           ` Dov Murik
2021-11-05 18:32       ` Dov Murik
2021-11-08 21:22         ` Brijesh Singh
2021-11-09  7:34           ` Dov Murik
2021-11-03 16:10     ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24823e7a-b4e6-150c-df41-a373850301a7@amd.com \
    --to=thomas.lendacky@amd.com \
    --cc=ashish.kalra@amd.com \
    --cc=brijesh.singh@amd.com \
    --cc=dgilbert@redhat.com \
    --cc=dovmurik@linux.ibm.com \
    --cc=ehabkost@redhat.com \
    --cc=jejb@linux.ibm.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=tobin@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.