All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Dov Murik <dovmurik@linux.ibm.com>
Cc: "Eduardo Habkost" <eduardo@habkost.net>,
	"Tom Lendacky" <thomas.lendacky@amd.com>,
	"Ashish Kalra" <Ashish.Kalra@amd.com>,
	"Brijesh Singh" <brijesh.singh@amd.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org, "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH 0/2] Improved support for AMD SEV firmware loading
Date: Mon, 17 Jan 2022 11:56:33 +0000	[thread overview]
Message-ID: <YeVZcQ/MW+P9Lh0l@redhat.com> (raw)
In-Reply-To: <59d81ace-8a66-4ab4-2768-a68d302e62d8@linux.ibm.com>

On Mon, Jan 17, 2022 at 09:34:30AM +0200, Dov Murik wrote:
> [+cc Tom, Brijesh, Ashish - see SEV-related changes in this series]
> 
> 
> On 13/01/2022 18:55, Daniel P. Berrangé wrote:
> > The AMD SEV build of EDK2 only emits a single file, intended to be
> > 
> > mapped readonly. There is explicitly no separate writable VARS
> > 
> > store for persisting non-volatile firmware variables.
> > 
> > 
> > 
> > This can be used with QEMU's traditional pflash configuration
> > 
> > mechanism by only populating pflash0, leaving pflash1 unconfigured.
> > 
> > Conceptually, however, it is odd to be using pflash at all when we
> > 
> > have no intention of supporting any writable variables. The -bios
> > 
> > option should be sufficient for any firmware that is exclusively
> > 
> > readonly code.
> > 
> > 
> > 
> > 
> > 
> > A second issue is that the firmware descriptor schema does not allow
> > 
> > for describing a firmware that uses pflash, without any associated
> > 
> > non-volatile storage.
> > 
> > 
> > 
> > In docs/interop/firmware.json
> > 
> > 
> > 
> >  'struct' : 'FirmwareMappingFlash',
> > 
> >   'data'   : { 'executable'     : 'FirmwareFlashFile',
> > 
> >                'nvram-template' : 'FirmwareFlashFile' } }
> > 
> > 
> > 
> > Notice that nvram-template is mandatory, and when consuming these
> > 
> > files libvirt will thus complain if the nvram-template field is
> > 
> > missing.
> > 
> > 
> > 
> > We could in theory make nvram-template optional in the schema and
> > 
> > then update libvirt to take account of it, but this feels dubious
> > 
> > when we have a perfectly good way of describing a firmware without
> > 
> > NVRAM, using 'FirmwareMappingMemory' which is intended to be used
> > 
> > with QEMU's -bios option.
> > 
> > 
> > 
> > 
> > 
> > A third issue is in libvirt, where again the code handling the
> > 
> > configuration of pflash supports two scenarios
> > 
> > 
> > 
> >  - A single pflash image, which is writable
> > 
> >  - A pair of pflash images, one writable one readonly
> > 
> > 
> > 
> > There is no support for a single read-only pflash image in libvirt
> > 
> > today.
> > 
> > 
> > 
> > 
> > 
> > This all points towards the fact that we should be using -bios
> > 
> > to load the AMD SEV firmware build of EDK.
> > 
> > 
> > 
> > The only thing preventing us doing that is that QEMU does not
> > 
> > initialize the SEV firmware when using -bios. That is fairly
> > 
> > easily solved, as done in this patch series.
> > 
> > 
> > 
> > For testing I've launched QEMU in in these scenarios
> > 
> > 
> > 
> >   - SEV guest using -bios and boot from HD
> > 
> >   - SEV guest using pflash and boot from HD
> > 
> >   - SEV-ES guest using -bios and direct kernel boot
> > 
> >   - SEV-ES guest using pflash and direct kernel boot
> > 
> > 
> > 
> > In all these cases I was able to validate the reported SEV
> > 
> > guest measurement.
> > 
> > 
> 
> 
> I'm having trouble testing this series (applied on top of master commit 69353c332c):
> it hangs with -bios but works OK with pflash:
> 
> Here's with -bios:
> 
> $ sudo /home/dmurik/git/qemu/build/qemu-system-x86_64 -enable-kvm \
>        -cpu host -machine q35 -smp 4 -m 2G \
>        -machine confidential-guest-support=sev0 \
>        -object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=1,policy=0x0 \
>        -bios /home/dmurik/git/edk2/Build/AmdSev/DEBUG_GCC5/FV/OVMF.fd \
>        -nographic \
>        -global isa-debugcon.iobase=0x402 -debugcon file:ovmf-1.log \
>        -monitor pty -trace 'enable=kvm_sev_*'
> 
> char device redirected to /dev/pts/14 (label compat_monitor0)
> kvm_sev_init
> kvm_sev_launch_start policy 0x0 session (nil) pdh (nil)
> kvm_sev_change_state uninit -> launch-update
> kvm_sev_launch_update_data addr 0x7f42e9bff010 len 0x400000
> kvm_sev_change_state launch-update -> launch-secret
> kvm_sev_launch_measurement data PF6n7+Vujx5sW8PC6iMRtHXfpXdJ4osbcfYvoknu7gg4ypMqs727NTzG86Ft8Llu
> kvm_sev_launch_finish
> kvm_sev_change_state launch-secret -> running
> 
> 
> Here it hangs. The ovmf-1.log file is empty.
> 
> Notice that kvm_sev_launch_update_data is called, so the new
> -bios behaviour is triggered correctly.  But the guest doesn't
> start running.

Sorry, it seems I failed to test this properly before rushing into
sending last week. I was too focused on making sure the SEV measurement
was working and not the rest.

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 :|



  reply	other threads:[~2022-01-17 11:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 16:55 [PATCH 0/2] Improved support for AMD SEV firmware loading Daniel P. Berrangé
2022-01-13 16:55 ` [PATCH 1/2] hw/i386: refactor logic for setting up SEV firmware Daniel P. Berrangé
2022-01-13 16:55 ` [PATCH 2/2] hw/i386: support loading OVMF using -bios too Daniel P. Berrangé
2022-01-14 18:07   ` Michael S. Tsirkin
2022-01-16 21:05     ` Philippe Mathieu-Daudé via
2022-01-17  8:53       ` Michael S. Tsirkin
2022-01-17  7:34 ` [PATCH 0/2] Improved support for AMD SEV firmware loading Dov Murik
2022-01-17 11:56   ` Daniel P. Berrangé [this message]
2022-01-17 12:12   ` Brijesh Singh
2022-01-20 10:15     ` 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=YeVZcQ/MW+P9Lh0l@redhat.com \
    --to=berrange@redhat.com \
    --cc=Ashish.Kalra@amd.com \
    --cc=brijesh.singh@amd.com \
    --cc=dgilbert@redhat.com \
    --cc=dovmurik@linux.ibm.com \
    --cc=eduardo@habkost.net \
    --cc=f4bug@amsat.org \
    --cc=kraxel@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thomas.lendacky@amd.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.