Re: [PATCH] qapi, i386/sev: Add debug-launch-digest to launch-measure response

2022-02-01 Thread Tobin Feldman-Fitzthum



On 1/31/22 9:26 AM, Daniel P. Berrangé wrote:

> 
> Ok, so the usage scenario is that the platform owner is deciding 
> which OVMF build in use, not the guest owner. That guest owner just
> knows that it is an OVMF build from a set of builds published by the
> platform owner. Good enough if you trust the cloud owner in general,
> but want confidence that their compute host isn't compromised. Would
> need  an independantly reproducible build, if you don't trust the
> cloud owner at all.
> 
> 
> Assuming we've got 5 possible OVMF builds, currently we would need
> to calculate 5 HMACs over the inpuot data.
> 
> With this extra piece of info, we only need to calculate 1 HMAC.
> 
> So this is enabling a performance optimization, that could indeed
> be used in a production deployment.  The HMAC ops are not exactly
> performance intensive though until we get to the point of choosing
> between a huge number of possible OVMFs.
> 
> If we can't get the VMSA info included, then the guest owner still
> needs a local copy of every possible OVMF binary that is valid. IOW
> this digest is essentially no more than a filename to identify which
> OVMF binary to calc the HMAC over.

For us the guest owner isn't monolithic. The guest owner will rely on a
Key Broker Service (KBS) running in some trusted domain to process
individual measurements and secret requests. The guest owner must
provision the KBS at the start of the day. I mention this because while
the guest owner will definitely need to build their own version of OVMF
and the components accounted for in the hashes table, ideally the
binaries won't need to be uploaded to the KBS.

If we forget about SEV-ES, the flow is pretty easy. At the start of the
day the guest owner builds the firmware, kernel, etc that they expect
the VM to be started with. They use a script to hash all the components,
formulate the hashes table, and ultimately produce the launch digest.
The guest owner uploads the launch digest to the KBS. If multiple
configurations are supported, they upload multiple digests.

We could do something very similar for SEV-ES, but there are some drawbacks.
> 
> IOW, I think there's only two scenarios that make sense
> 
> 1. The combined launch digest over firmware, kernel hashes
>and VMSA state.
> 
> 2. Individual hashes for each of firmware, kernel hashes table and
>VMSA state
> 
> I think we should assume that anyone who has access to SEV-ES hardware
> is likely to run in SEV-ES policy, not SEV policy. So without VMSA
> state I think that (1) is of quite limited usefulness. (2) would
> be nicer to allow optimization of picking which OVMF blob to use,
> as you wouldn't need to figure out the cross-product of very valid
> OVMF and every valid kernel hashes table - the latter probably isn't
> even a finite bounded set.
> 
I see something very similar. We could support -ES by doing the same
thing described above for SEV. The catch is that the guest owner would
have to calculate a firmware digest for every possible VMSA. That said,
if we assume that the VMSAs aren't going to change much, this just comes
down to a different VMSA and launch digest for each allowed cpu count.

Generating these digests could probably be automatically handled by the
same script that generates the secret table and calculates the launch
digest. No matter what we do, the guest owner will need to come up with
an expected VMSA. We would probably have to extend QEMU to include the
VMSA in the debug hash field for approach this to work.

>> More generally: within space/network constraints, give the Guest Owner
>> all the information it needs to compute the launch measurement.  There's
>> a problem with OVMF there (we don't want to send the whole 4 MB over the
>> QMP response, but given its hash we can't "extend" it to include the
>> kernel-hashes struct).
> 
> Yeah its a shame we aren't just measuring the digest of each piece
> of information in  GCTX.LD, instead of measuring the raw information
> directly.
Exactly. This is annoying because it complicates the other option, which
is to have the guest owner upload hashes of each individual component to
the KBS, which would generate the final hash when checking a launch
measurement. Unfortunately the launch digest is the hash of the firmware
binary itself, not the hash of the hash of the firmware. This means that
the guest owner would have to upload the firmware binary to the KBS. If
we took this approach the guest owner wouldn't have to generate a bunch
of launch digests, although they might need to provide the KBS with
complex instructions about which components can be used together.

Anyway, I think either of these options is fine. The first is more work
for the guest owner at configuration time and the second is more work
for the KBS at runtime. Like I said, the guest owner will need to know
an expected VMSA either way so maybe we should think of that as a
separate issue.
> 
> 
> I wonder if we're thinking of this at the wrong level 

Re: Fw: [EXTERNAL] Re: [RFC PATCH 00/13] Add support for Mirror VM.

2021-08-24 Thread Tobin Feldman-Fitzthum
On Mon, Aug 16, 2021 at 04:15:46PM +0200, Paolo Bonzini wrote:

> Hi,
>
> first of all, thanks for posting this work and starting the discussion.
>
> However, I am not sure if the in-guest migration helper vCPUs should use
> the existing KVM support code.  For example, they probably can just
> always work with host CPUID (copied directly from
> KVM_GET_SUPPORTED_CPUID), and they do not need to interface with QEMU's
> MMIO logic.  They would just sit on a "HLT" instruction and communicate
> with the main migration loop using some kind of standardized ring buffer
> protocol; the migration loop then executes KVM_RUN in order to start the
> processing of pages, and expects a KVM_EXIT_HLT when the VM has nothing
> to do or requires processing on the host.
> The migration helper can then also use its own address space, for
> example operating directly on ram_addr_t values with the helper running
> at very high virtual addresses.  Migration code can use a
> RAMBlockNotifier to invoke KVM_SET_USER_MEMORY_REGION on the mirror VM
> (and never enable dirty memory logging on the mirror VM, too, which has
> better performance).
>
> With this implementation, the number of mirror vCPUs does not even have
> to be indicated on the command line.  The VM and its vCPUs can simply be
> created when migration starts.  In the SEV-ES case, the guest can even
> provide the VMSA that starts the migration helper.

It might make sense to tweak the mirror support code so that it is more
closely tied to migration and the migration handler. On the other hand,
the usage of a mirror VM might be more general than just migration. In
some ways the mirror offers similar functionality to the VMPL in SNP,
providing a way to run non-workload code inside the enclave. This
potentially has uses beyond migration. If this is the case, do maybe we
want to keep the mirror more general.

It's also worth noting that the SMP interface that Ashish is using to
specify the mirror might come in handy if we ever want to have more than
one vCPU in the mirror. For instance we might want to use multiple MH
vCPUs to increase throughput.

-Tobin

> The disadvantage is that, as you point out, in the future some of the
> infrastructure you introduce might be useful for VMPL0 operation on
> SEV-SNP.  My proposal above might require some code duplication.
> However, it might even be that VMPL0 operation works best with a model
> more similar to my sketch of the migration helper; it's really too early
> to say.
>
> Paolo



Re: [RFC PATCH 00/13] Add support for Mirror VM.

2021-08-23 Thread Tobin Feldman-Fitzthum

On 8/23/21 8:26 AM, Dr. David Alan Gilbert wrote:


* James Bottomley (j...@linux.ibm.com) wrote:


(is there an attest of the destination happening here?)
There will be in the final version.  The attestations of the source and
target, being the hash of the OVMF (with the registers in the -ES
case), should be the same (modulo any firmware updates to the PSP,
whose firmware version is also hashed) to guarantee the OVMF is the
same on both sides.  We'll definitely take an action to get QEMU to
verify this ... made a lot easier now we have signed attestations ...

Hmm; I'm not sure you're allowed to have QEMU verify that - we don't
trust it; you need to have either the firmware say it's OK to migrate
to the destination (using the existing PSP mechanism) or get the source
MH to verify a quote from the destination.


I think the check in QEMU would only be a convenience. The launch 
measurement of the target (verified by the guest owner) is what 
guarantees that the firmware, as well as the policy, of the target is 
what is expected. In PSP-assisted migration the source verifies the 
target, but our plan is to have the guest owner verify both the source 
and the target. The target will only be provisioned with the transport 
key if the measurement checks out. We will have some more details about 
this key agreement scheme soon.



[Somewhere along the line, if you're not using the PSP, I think you also
need to check the guest policy to check it is allowed to migrate].


Sources that aren't allowed to migrate won't be provisioned with 
transport key to encrypt pages. A non-migrateable guest could also be 
booted with OvmfPkg firmware, which does not contain the migration handler.


-Tobin


Dave


James






Re: [RFC PATCH 00/13] Add support for Mirror VM.

2021-08-19 Thread Tobin Feldman-Fitzthum

On 8/19/21 4:22 AM, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.ibm.com) wrote:

On 8/18/21 3:04 PM, Dr. David Alan Gilbert wrote:


Are you relying on the target firmware to be *identical* or purely for
it to be *compatible* ?  It's normal for a migration to be the result of
wanting to do an upgrade; and that means the destination build of OVMF
might be newer (or older, or ...).

Dave

This is a good point. The migration handler on the source and target must
have the same memory footprint or bad things will happen. Using the same
firmware on the source and target is an easy way to guarantee this. Since
the MH in OVMF is not a contiguous region of memory, but a group of
functions scattered around OVMF, it is a bit difficult to guarantee that the
memory footprint is the same if the build is different.

Can you explain what the 'memory footprint' consists of? Can't it just
be the whole of the OVMF rom space if you have no way of nudging the MH
into it's own chunk?


The footprint is not massive. It is mainly ConfidentialMigrationDxe and 
the OVMF crypto support. It might be feasible to copy these components 
to a fixed location that would be the same across fw builds. It might 
also be feasible to pin these components to certain addresses. OVMF sort 
of supports doing this. We can raise the question in that community.


It also might work to protect the entirety of OVMF as you suggest. 
Currently we don't copy any of the OVMF ROM (as in flash0) over. That 
said, the MH doesn't run from the ROM so we would need to protect the 
memory used by OVMF as well. In some ways it might seem easier to 
protect all of the OVMF memory rather than just a couple of packages, 
but there are some complexities. For one thing, we would only want to 
protect efi runtime memory, as boot memory may be in use by the OS and 
would need to be migrated. The MH could check whether each page is efi 
runtime memory and skip any pages that are. Runtime memory won't be a 
contiguous blob, however, so for this approach the layout of the runtime 
memory would need to be the same on the source and target.


We can sidestep these issues entirely by using identical firmware 
images. That said, there are a number of strategies for developing 
compatible OVMF images and I definitely see the value of doing so.


-Tobin



I think it really does have to cope with migration to a new version of
host.

Dave


-Tobin




We start the target like a normal VM rather than
waiting for an incoming migration. The plan is to treat the target like a
normal VM for attestation as well. The guest owner will attest the target VM
just like they would any other VM that is started on their behalf. Secret
injection can be used to establish a shared key for the source and target.

-Tobin


--Steve





Re: [RFC PATCH 00/13] Add support for Mirror VM.

2021-08-18 Thread Tobin Feldman-Fitzthum

On 8/18/21 3:04 PM, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.ibm.com) wrote:

On 8/17/21 6:04 PM, Steve Rutherford wrote:

Ahh, It sounds like you are looking into sidestepping the existing
AMD-SP flows for migration. I assume the idea is to spin up a VM on
the target side, and have the two VMs attest to each other. How do the
two sides know if the other is legitimate? I take it that the source
is directing the LAUNCH flows?

Yeah we don't use PSP migration flows at all. We don't need to send the MH
code from the source to the target because the MH lives in firmware, which
is common between the two.

Are you relying on the target firmware to be *identical* or purely for
it to be *compatible* ?  It's normal for a migration to be the result of
wanting to do an upgrade; and that means the destination build of OVMF
might be newer (or older, or ...).

Dave


This is a good point. The migration handler on the source and target 
must have the same memory footprint or bad things will happen. Using the 
same firmware on the source and target is an easy way to guarantee this. 
Since the MH in OVMF is not a contiguous region of memory, but a group 
of functions scattered around OVMF, it is a bit difficult to guarantee 
that the memory footprint is the same if the build is different.


-Tobin





We start the target like a normal VM rather than
waiting for an incoming migration. The plan is to treat the target like a
normal VM for attestation as well. The guest owner will attest the target VM
just like they would any other VM that is started on their behalf. Secret
injection can be used to establish a shared key for the source and target.

-Tobin


--Steve





Re: [RFC PATCH 00/13] Add support for Mirror VM.

2021-08-18 Thread Tobin Feldman-Fitzthum

On 8/17/21 6:04 PM, Steve Rutherford wrote:

On Tue, Aug 17, 2021 at 1:50 PM Tobin Feldman-Fitzthum
 wrote:

This is essentially what we do in our prototype, although we have an
even simpler approach. We have a 1:1 mapping that maps an address to
itself with the cbit set. During Migration QEMU asks the migration
handler to import/export encrypted pages and provides the GPA for said
page. Since the migration handler only exports/imports encrypted pages,
we can have the cbit set for every page in our mapping. We can still use
OVMF functions with these mappings because they are on encrypted pages.
The MH does need to use a few shared pages (to communicate with QEMU,
for instance), so we have another mapping without the cbit that is at a
large offset.

I think this is basically equivalent to what you suggest. As you point
out above, this approach does require that any page that will be
exported/imported by the MH is mapped in the guest. Is this a bad
assumption? The VMSA for SEV-ES is one example of a region that is
encrypted but not mapped in the guest (the PSP handles it directly). We
have been planning to map the VMSA into the guest to support migration
with SEV-ES (along with other changes).

Ahh, It sounds like you are looking into sidestepping the existing
AMD-SP flows for migration. I assume the idea is to spin up a VM on
the target side, and have the two VMs attest to each other. How do the
two sides know if the other is legitimate? I take it that the source
is directing the LAUNCH flows?


Yeah we don't use PSP migration flows at all. We don't need to send the 
MH code from the source to the target because the MH lives in firmware, 
which is common between the two. We start the target like a normal VM 
rather than waiting for an incoming migration. The plan is to treat the 
target like a normal VM for attestation as well. The guest owner will 
attest the target VM just like they would any other VM that is started 
on their behalf. Secret injection can be used to establish a shared key 
for the source and target.


-Tobin



--Steve





Re: [RFC PATCH 00/13] Add support for Mirror VM.

2021-08-17 Thread Tobin Feldman-Fitzthum



On 8/17/21 12:32 PM, Paolo Bonzini wrote:

On 17/08/21 01:53, Steve Rutherford wrote:

Separately, I'm a little weary of leaving the migration helper mapped
into the shared address space as writable.


A related question here is what the API should be for how the 
migration helper sees the memory in both physical and virtual address.


First of all, I would like the addresses passed to and from the 
migration helper to *not* be guest physical addresses (this is what I 
referred to as QEMU's ram_addr_t in other messages).  The reason is 
that some unmapped memory regions, such as virtio-mem hotplugged 
memory, would still have to be transferred and could be encrypted.  
While the guest->host hypercall interface uses guest physical 
addresses to communicate which pages are encrypted, the host can do 
the GPA->ram_addr_t conversion and remember the encryption status of 
currently-unmapped regions.


This poses a problem, in that the guest needs to prepare the page 
tables for the migration helper and those need to use the migration 
helper's physical address space.


There's three possibilities for this:

1) the easy one: the bottom 4G of guest memory are mapped in the 
mirror VM 1:1.  The ram_addr_t-based addresses are shifted by either 
4G or a huge value such as 2^42 (MAXPHYADDR - physical address 
reduction - 1). This even lets the migration helper reuse the OVMF 
runtime services memory map (but be careful about thread safety...).


This is essentially what we do in our prototype, although we have an 
even simpler approach. We have a 1:1 mapping that maps an address to 
itself with the cbit set. During Migration QEMU asks the migration 
handler to import/export encrypted pages and provides the GPA for said 
page. Since the migration handler only exports/imports encrypted pages, 
we can have the cbit set for every page in our mapping. We can still use 
OVMF functions with these mappings because they are on encrypted pages. 
The MH does need to use a few shared pages (to communicate with QEMU, 
for instance), so we have another mapping without the cbit that is at a 
large offset.


I think this is basically equivalent to what you suggest. As you point 
out above, this approach does require that any page that will be 
exported/imported by the MH is mapped in the guest. Is this a bad 
assumption? The VMSA for SEV-ES is one example of a region that is 
encrypted but not mapped in the guest (the PSP handles it directly). We 
have been planning to map the VMSA into the guest to support migration 
with SEV-ES (along with other changes).


2) the more future-proof one.  Here, the migration helper tells QEMU 
which area to copy from the guest to the mirror VM, as a (main GPA, 
length, mirror GPA) tuple.  This could happen for example the first 
time the guest writes 1 to MSR_KVM_MIGRATION_CONTROL.  When migration 
starts, QEMU uses this information to issue KVM_SET_USER_MEMORY_REGION 
accordingly.  The page tables are built for this (usually very high) 
mirror GPA and the migration helper operates in a completely separate 
address space.  However, the backing memory would still be shared 
between the main and mirror VMs.  I am saying this is more future 
proof because we have more flexibility in setting up the physical 
address space of the mirror VM.


The Migration Handler in OVMF is not a contiguous region of memory. The 
MH uses OVMF helper functions that are allocated in various regions of 
runtime memory. I guess I can see how separating the memory of the MH 
and the guest OS could be positive. On the other hand, since the MH is 
in OVMF, it is fundamentally designed to coexist with the guest OS.


What do you envision in terms of future changes to the mirror address space?

3) the paranoid one, which I think is what you hint at above: this is 
an extension of (2), where userspace invokes the PSP send/receive API 
to copy the small requested area of the main VM into the mirror VM.  
The mirror VM code and data are completely separate from the main VM.  
All that the mirror VM shares is the ram_addr_t data. Though I am not 
even sure it is possible to use the send/receive API this way...


Yeah not sure if you could use the PSP for this.

-Tobin



What do you think?

Paolo






Re: [RFC PATCH 00/13] Add support for Mirror VM.

2021-08-16 Thread Tobin Feldman-Fitzthum

On Mon, Aug 16 at 10:44 AM Ashish Kalra wrote:

> I am not sure if we really don't need QEMU's MMIO logic, I think that 
once the>
> mirror VM starts booting and running the UEFI code, it might be only 
during

> the PEI or DXE phase where it will start actually running the MH code,
> so mirror VM probably still need to handles KVM_EXIT_IO when SEC 
phase does I/O,
> I can see PIC accesses and Debug Agent initialization stuff in SEC 
startup code.


The migration handler prototype that we are releasing in the near future 
does not use the SEC or PEI phases in the mirror. We have some support 
code that runs in the main VM and sets up the migration handler entry 
point. QEMU starts the mirror pointing to this entry point, which does 
some more setup (like switching to long mode) and jumps to the migration 
handler.


-Tobin

> Addtionally this still requires CPUState{..} structure and the backing
> "X86CPU" structure, for example, as part of kvm_arch_post_run() to get
> the MemTxAttrs needed by kvm_handle_io().

> Thanks,
> Ashish




Re: RFC: Fast Migration for SEV and SEV-ES - blueprint and proof of concept

2020-10-30 Thread Tobin Feldman-Fitzthum

On 2020-10-30 16:02, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.ibm.com) wrote:

Hello,

Dov Murik, James Bottomley, Hubertus Franke, and I have been working 
on a
plan for fast live migration with SEV and SEV-ES. We just posted an 
RFC
about it to the edk2 list. It includes a proof-of-concept for what we 
feel

to be the most difficult part of fast live migration with SEV-ES.

https://edk2.groups.io/g/devel/topic/77875297

This was posted to the edk2 list because OVMF is one of the main 
components
of our approach to live migration. With SEV/SEV-ES the hypervisor 
generally

does not have access to guest memory/CPU state. We propose a Migration
Handler in OVMF that runs inside the guest and assists the hypervisor 
with
migration. One major challenge to this approach is that for SEV-ES 
this
Migration Handler must be able to set the CPU state of the target to 
the CPU
state of the source while the target is running. Our demo shows that 
this is

feasible.

While OVMF is a major component of our approach, QEMU obviously has a 
huge

part to play as well. We want to start thinking about the best way to
support fast live migration for SEV and for encrypted VMs in general. 
A
handful of issues are starting to come into focus. For instance, the 
target
VM needs to be started before we begin receiving pages from the source 
VM.


That might not be that hard to fudge; we already start the VM in
postcopy mode before we have all of RAM; now in that we have to do 
stuff

to protect the RAM because we expect the guest to access it; in this
case you possibly don't need to.


I'll need to think a bit about this. The basic setup is that we want the
VM to boot into OVMF and initialize the Migration Handler. Then QEMU 
will start
receiving encrypted pages and passing them into OVMF via some shared 
address.

The Migration Handler will decrypt the pages and put them into place,
overwriting everything around it. The Migration Handler will be a 
runtime

driver so it should avoid overwriting itself.

We will also need to start an extra vCPU for the Migration Handler to 
run
on. We are currently working on a demo of end-to-end live migration 
for SEV
(non-ES) VMs that should help crystallize these issues. It should be 
on the
list around the end of the year. For now, check out our other post, 
which

has a lot more information and let me know if you have any thoughts.


I don't think I understood why you'd want to map the VMSA, or why it
would be encrypted in such a way it was useful to you.


We map the VMSA into the guest because it gives us an easy way to
securely send the CPU state to the target.

Each time there is a VMExit, the CPU state of the guest
is stored in the VMSA. Although the VMSA is encrypted with the guest's 
key,

it usually isn't mapped into the guest. During migration we securely
transfer guest memory from source to destination (the Migration Handler
on source and target share a key which they use to encrypt/decrypt the
pages). If we tweak the NPT to add the VMSA to the guest, the VMSA will 
be

sent along with the all the other pages.

There are some details with the timing. We'll need to force a VMExit 
once
we get convergence and re-send the VMSA page to make sure it's up to 
date.
Once the target has all the pages, the Migration Handler can just read 
the

CPU state from some known address.

-Tobin


Dave



-Tobin





RFC: Fast Migration for SEV and SEV-ES - blueprint and proof of concept

2020-10-30 Thread Tobin Feldman-Fitzthum

Hello,

Dov Murik, James Bottomley, Hubertus Franke, and I have been working on 
a plan for fast live migration with SEV and SEV-ES. We just posted an 
RFC about it to the edk2 list. It includes a proof-of-concept for what 
we feel to be the most difficult part of fast live migration with SEV-ES.


https://edk2.groups.io/g/devel/topic/77875297

This was posted to the edk2 list because OVMF is one of the main 
components of our approach to live migration. With SEV/SEV-ES the 
hypervisor generally does not have access to guest memory/CPU state. We 
propose a Migration Handler in OVMF that runs inside the guest and 
assists the hypervisor with migration. One major challenge to this 
approach is that for SEV-ES this Migration Handler must be able to set 
the CPU state of the target to the CPU state of the source while the 
target is running. Our demo shows that this is feasible.


While OVMF is a major component of our approach, QEMU obviously has a 
huge part to play as well. We want to start thinking about the best way 
to support fast live migration for SEV and for encrypted VMs in general. 
A handful of issues are starting to come into focus. For instance, the 
target VM needs to be started before we begin receiving pages from the 
source VM. We will also need to start an extra vCPU for the Migration 
Handler to run on. We are currently working on a demo of end-to-end live 
migration for SEV (non-ES) VMs that should help crystallize these 
issues. It should be on the list around the end of the year. For now, 
check out our other post, which has a lot more information and let me 
know if you have any thoughts.


-Tobin



Re: [PATCH v7] sev: add sev-inject-launch-secret

2020-10-27 Thread Tobin Feldman-Fitzthum

On 2020-10-27 09:35, Eduardo Habkost wrote:

On Thu, Oct 22, 2020 at 01:39:09AM -0400, to...@linux.ibm.com wrote:

From: Tobin Feldman-Fitzthum 

AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Brijesh Singh 


I was going to queue it, but unfortunately it failed to build on some 
hosts:


https://gitlab.com/ehabkost/qemu/-/jobs/814250096

[1892/5203] Compiling C object 
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o

FAILED: libqemu-alpha-softmmu.fa.p/monitor_misc.c.o
arm-linux-gnueabi-gcc -Ilibqemu-alpha-softmmu.fa.p -I. -I..
-Itarget/alpha -I../target/alpha -I../capstone/include/capstone -Iqapi
-Itrace -Iui -Iui/shader -I/usr/include/libdrm -I/usr/include/pixman-1
-I/usr/include/glib-2.0 -I/usr/lib/arm-linux-gnueabi/glib-2.0/include
-fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu99
-O2 -g -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration
-Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
-Wendif-labels -Wexpansion-to-defined -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong
-DLEGACY_RDMA_REG_MR -isystem /builds/ehabkost/qemu/linux-headers
-isystem linux-headers -iquote /builds/ehabkost/qemu/tcg/arm -iquote .
-iquote /builds/ehabkost/qemu -iquote /builds/ehabkost/qemu/accel/tcg
-iquote /builds/ehabkost/qemu/include -iquote
/builds/ehabkost/qemu/disas/libvixl -pthread -fPIC
-isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H
'-DCONFIG_TARGET="alpha-softmmu-config-target.h"'
'-DCONFIG_DEVICES="alpha-softmmu-config-devices.h"' -MD -MQ
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o -MF
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o.d -o
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o -c ../monitor/misc.c
../monitor/misc.c: In function 'gpa2hva':
../monitor/misc.c:686:18: error: invalid operands to binary < (have
'Int128' {aka 'struct Int128'} and 'uint64_t' {aka 'long long unsigned
int'})
 if (mrs.size < size) {
  ^
[1893/5203] Compiling C object 
libqemu-alpha-softmmu.fa.p/softmmu_physmem.c.o

ninja: build stopped: subcommand failed.


I am not easily able to replicate this (perhaps an issue for ARM only?).

Either way, I think it would be better to make size into an Int128
and use the appropriate comparison function. I will submit a new 
version.

I can test this better with a bit more time. For now, up to you if you
want to try building it.

-Tobin



Re: [PATCH v6] sev: add sev-inject-launch-secret

2020-10-21 Thread Tobin Feldman-Fitzthum

On 2020-10-22 00:16, to...@linux.ibm.com wrote:

From: Tobin Feldman-Fitzthum 

AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Brijesh Singh 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c| 13 +---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 65 +++
 target/i386/trace-events  |  1 +
 8 files changed, 110 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
 #include "block/block.h"
 #include "qapi/qapi-types-misc.h"
 #include "qemu/readline.h"
+#include "include/exec/hwaddr.h"

 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);

+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error 
**errp);

+
 void monitor_read_command(MonitorHMP *mon, int show_prompt);
 int monitor_read_password(MonitorHMP *mon, ReadLineFunc 
*readline_func,

   void *opaque);
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..7ab6e3e31d 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@

 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..dd148be5da 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,10 @@ static void hmp_physical_memory_dump(Monitor
*mon, const QDict *qdict)
 memory_dump(mon, count, format, size, addr, 1);
 }

-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error 
**errp)

 {
 MemoryRegionSection mrs = memory_region_find(get_system_memory(),
- addr, 1);
+ addr, size);

 if (!mrs.mr) {
 error_setg(errp, "No memory is mapped at address 0x%"
HWADDR_PRIx, addr);
@@ -683,6 +683,11 @@ static void *gpa2hva(MemoryRegion **p_mr, hwaddr
addr, Error **errp)
 return NULL;
 }

+if (mrs.size < size) {
+error_setg(errp, "Size of memory region at 0x%" HWADDR_PRIx
+   " exceeded.", addr);
+}
+


Forgot to return :( Will update.


 *p_mr = mrs.mr;
 return qemu_map_ram_ptr(mrs.mr->ram_block, 
mrs.offset_within_region);

 }
@@ -694,7 +699,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict 
*qdict)

 MemoryRegion *mr = NULL;
 void *ptr;

-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
@@ -770,7 +775,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict 
*qdict)

 void *ptr;
 uint64_t physaddr;

-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,24 @@
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
   'if': 'defined(TARGET_I386)' }

+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 5.2
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' 
},

+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -728,3 +728,10 @@ SevCapability *qmp_query_sev_capabilities(Error 
**errp)

 {
 return sev_get_capabilities(errp);
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+

Re: [PATCH v5] sev: add sev-inject-launch-secret

2020-10-20 Thread Tobin Feldman-Fitzthum

On 2020-10-20 11:56, Paolo Bonzini wrote:

On 20/10/20 15:54, Eduardo Habkost wrote:

On Tue, Oct 20, 2020 at 11:03:51AM +0200, Paolo Bonzini wrote:

On 15/10/20 16:37, to...@linux.ibm.com wrote:
-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error 
**errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, 
Error **errp)

 {
 MemoryRegionSection mrs = 
memory_region_find(get_system_memory(),

- addr, 1);
+ addr, size);


You need to check size against mrs.size and fail if mrs.size is 
smaller.

 Otherwise, the ioctl can access memory out of range.


Good catch!  I'm dequeuing it.

Is there a reason memory_region_find() doesn't ensure that by
default?


IIRC memory_region_find() was used to do DMA in the very first versions
of "virtio-blk dataplane" so you would call it multiple times in a 
loop.

 So it's like that because it maps the way address_space_map() works.


The call at virtio_balloon_handle_output() looks suspicious,
though, because it looks for a BALLOON_PAGE_SIZE range, but
there's no check for the returned section size.


I think it's not a bug because ultimately it's checked in
ram_block_discard_range, but it's not pretty.

Paolo


Ok, it seems like the best solution is, as Paolo suggested, to add a
simple check at the end of gpa2hva that makes sure mr.size is equal
to size. gpa2hva is used one other place where the size is hard-coded
as 1, so adding the check isn't going to break anything.

Would you like me to resubmit with this tweak?

-Tobin



Re: [PATCH v5] sev: add sev-inject-launch-secret

2020-10-19 Thread Tobin Feldman-Fitzthum

On 2020-10-19 12:47, Eduardo Habkost wrote:

On Mon, Oct 19, 2020 at 12:46:08PM -0400, Eduardo Habkost wrote:

On Thu, Oct 15, 2020 at 10:37:13AM -0400, to...@linux.ibm.com wrote:
[...]
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index 88e3f39a1e..2d2ee54cc6 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -49,3 +49,8 @@ SevCapability *sev_get_capabilities(Error **errp)
>  error_setg(errp, "SEV is not available in this QEMU");
>  return NULL;
>  }
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa)
> +{
> +return 1;
> +}

This doesn't match the actual function prototype.  I had to apply the 
following

fixup:

---
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 2d2ee54cc6..62a2587e7b 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -49,8 +49,10 @@ SevCapability *sev_get_capabilities(Error **errp)
 error_setg(errp, "SEV is not available in this QEMU");
 return NULL;
 }
+
 int sev_inject_launch_secret(const char *hdr, const char *secret,
- uint64_t gpa)
+ uint64_t gpa, Error *errp)


Oops. Fixing up the fixup:


Thanks Eduardo.

-Tobin


---
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 62a2587e7b..e4e60d9a7d 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -51,7 +51,7 @@ SevCapability *sev_get_capabilities(Error **errp)
 }

 int sev_inject_launch_secret(const char *hdr, const char *secret,
- uint64_t gpa, Error *errp)
+ uint64_t gpa, Error **errp)
 {
 error_setg(errp, "SEV is not available in this QEMU");
 return 1;




Re: [PATCH v4] sev: add sev-inject-launch-secret

2020-10-14 Thread Tobin Feldman-Fitzthum

On 2020-10-14 11:42, Brijesh Singh wrote:

On 10/14/20 10:17 AM, to...@linux.ibm.com wrote:

From: Tobin Feldman-Fitzthum 

AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 +++---
 qapi/misc-target.json | 18 
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 
 target/i386/sev.c | 60 
+++

 target/i386/trace-events  |  1 +
 8 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
 #include "block/block.h"
 #include "qapi/qapi-types-misc.h"
 #include "qemu/readline.h"
+#include "include/exec/hwaddr.h"

 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);

+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error 
**errp);

+
 void monitor_read_command(MonitorHMP *mon, int show_prompt);
 int monitor_read_password(MonitorHMP *mon, ReadLineFunc 
*readline_func,

   void *opaque);
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..7ab6e3e31d 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@

 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..f1ade245d5 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,10 @@ static void hmp_physical_memory_dump(Monitor 
*mon, const QDict *qdict)

 memory_dump(mon, count, format, size, addr, 1);
 }

-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error 
**errp)

 {
 MemoryRegionSection mrs = memory_region_find(get_system_memory(),
- addr, 1);
+ addr, size);

 if (!mrs.mr) {
 error_setg(errp, "No memory is mapped at address 0x%" 
HWADDR_PRIx, addr);
@@ -694,7 +694,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict 
*qdict)

 MemoryRegion *mr = NULL;
 void *ptr;

-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
@@ -770,7 +770,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict 
*qdict)

 void *ptr;
 uint64_t physaddr;

-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,24 @@
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
   'if': 'defined(TARGET_I386)' }

+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 5.2
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' 
},

+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -728,3 +728,10 @@ SevCapability *qmp_query_sev_capabilities(Error 
**errp)

 {
 return sev_get_capabilities(errp);
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 88e3f39a1e..2d2ee54cc6 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -49,3 +49,8 @@ SevCapability *sev_get_capabilities(Error **errp)
 error_setg(errp, "SEV is not available in this QEMU");
 

Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret

2020-09-21 Thread Tobin Feldman-Fitzthum

On 2020-09-21 15:16, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.vnet.ibm.com) wrote:

AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU faciliates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 


Hi Tobin,
  Did the ovmf stuff for agreeing the GUID for automating this ever
happen?


OVMF patches have not been upstreamed yet. I think we are planning
to do that relatively soon.

-Tobin

Dave


---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 ++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  9 ++
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 66 
+++

 target/i386/trace-events  |  1 +
 8 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1018d754a6..bf049c5b00 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
 #include "block/block.h"
 #include "qapi/qapi-types-misc.h"
 #include "qemu/readline.h"
+#include "include/exec/hwaddr.h"

 extern __thread Monitor *cur_mon;
 typedef struct MonitorHMP MonitorHMP;
@@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
 int monitor_get_cpu_index(void);

+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error 
**errp);

+
 void monitor_read_command(MonitorHMP *mon, int show_prompt);
 int monitor_read_password(MonitorHMP *mon, ReadLineFunc 
*readline_func,

   void *opaque);
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..b279b293e8 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@

 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 89bb970b00..b9ec8ba410 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor 
*mon, const QDict *qdict)

 memory_dump(mon, count, format, size, addr, 1);
 }

-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error 
**errp)

 {
 MemoryRegionSection mrs = memory_region_find(get_system_memory(),
- addr, 1);
+ addr, size);

 if (!mrs.mr) {
 error_setg(errp, "No memory is mapped at address 0x%" 
HWADDR_PRIx, addr);
@@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict 
*qdict)

 MemoryRegion *mr = NULL;
 void *ptr;

-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
@@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict 
*qdict)

 void *ptr;
 uint64_t physaddr;

-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index dee3b45930..d145f916b3 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,24 @@
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
   'if': 'defined(TARGET_I386)' }

+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 5.1
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' 
},

+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 27ebfa3ad2..42bcfe6dc0 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error 
**errp)


 return data;
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
+error_setg(errp, "SEV inject secret failed");
+}
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index e5ee13309c..fed4588185 100644
--- a/

[PATCH v3] SEV: QMP support for Inject-Launch-Secret

2020-07-06 Thread Tobin Feldman-Fitzthum
AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU faciliates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 ++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  9 ++
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 66 +++
 target/i386/trace-events  |  1 +
 8 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1018d754a6..bf049c5b00 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
 #include "block/block.h"
 #include "qapi/qapi-types-misc.h"
 #include "qemu/readline.h"
+#include "include/exec/hwaddr.h"
 
 extern __thread Monitor *cur_mon;
 typedef struct MonitorHMP MonitorHMP;
@@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
 int monitor_get_cpu_index(void);
 
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp);
+
 void monitor_read_command(MonitorHMP *mon, int show_prompt);
 int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
   void *opaque);
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..b279b293e8 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@
 
 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 89bb970b00..b9ec8ba410 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor *mon, const 
QDict *qdict)
 memory_dump(mon, count, format, size, addr, 1);
 }
 
-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
 {
 MemoryRegionSection mrs = memory_region_find(get_system_memory(),
- addr, 1);
+ addr, size);
 
 if (!mrs.mr) {
 error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, 
addr);
@@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
 MemoryRegion *mr = NULL;
 void *ptr;
 
-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
@@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
 void *ptr;
 uint64_t physaddr;
 
-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index dee3b45930..d145f916b3 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,24 @@
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
   'if': 'defined(TARGET_I386)' }
 
+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 5.1
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 27ebfa3ad2..42bcfe6dc0 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
 
 return data;
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
+error_setg(errp, "SEV inject secret failed");
+}
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index e5ee13309c..fed4588185 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
 {
 return NULL;
 }
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa)
+{
+return 1;
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c

[PATCH v2] SEV: QMP support for Inject-Launch-Secret

2020-07-02 Thread Tobin Feldman-Fitzthum
From: Tobin Feldman-Fitzthum 

AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU faciliates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 ++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  9 ++
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 66 +++
 target/i386/sev_i386.h|  3 ++
 target/i386/trace-events  |  1 +
 9 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1018d754a6..bf049c5b00 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
 #include "block/block.h"
 #include "qapi/qapi-types-misc.h"
 #include "qemu/readline.h"
+#include "include/exec/hwaddr.h"
 
 extern __thread Monitor *cur_mon;
 typedef struct MonitorHMP MonitorHMP;
@@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
 int monitor_get_cpu_index(void);
 
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp);
+
 void monitor_read_command(MonitorHMP *mon, int show_prompt);
 int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
   void *opaque);
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..b279b293e8 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@
 
 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 89bb970b00..b9ec8ba410 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor *mon, const 
QDict *qdict)
 memory_dump(mon, count, format, size, addr, 1);
 }
 
-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
 {
 MemoryRegionSection mrs = memory_region_find(get_system_memory(),
- addr, 1);
+ addr, size);
 
 if (!mrs.mr) {
 error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, 
addr);
@@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
 MemoryRegion *mr = NULL;
 void *ptr;
 
-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
@@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
 void *ptr;
 uint64_t physaddr;
 
-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index dee3b45930..d145f916b3 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,24 @@
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
   'if': 'defined(TARGET_I386)' }
 
+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 5.1
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 27ebfa3ad2..42bcfe6dc0 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
 
 return data;
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
+error_setg(errp, "SEV inject secret failed");
+}
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index e5ee13309c..fed4588185 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
 {
 return NULL;
 }
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa)
+{
+ 

[PATCH 1/1] SEV: QMP support for Inject-Launch-Secret

2020-06-30 Thread Tobin Feldman-Fitzthum
From: Tobin Feldman-Fitzthum 

AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU faciliates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 ++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  9 ++
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 66 +++
 target/i386/trace-events  |  1 +
 8 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1018d754a6..bf049c5b00 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
 #include "block/block.h"
 #include "qapi/qapi-types-misc.h"
 #include "qemu/readline.h"
+#include "include/exec/hwaddr.h"
 
 extern __thread Monitor *cur_mon;
 typedef struct MonitorHMP MonitorHMP;
@@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
 int monitor_get_cpu_index(void);
 
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp);
+
 void monitor_read_command(MonitorHMP *mon, int show_prompt);
 int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
   void *opaque);
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..b279b293e8 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@
 
 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 89bb970b00..b9ec8ba410 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor *mon, const 
QDict *qdict)
 memory_dump(mon, count, format, size, addr, 1);
 }
 
-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
 {
 MemoryRegionSection mrs = memory_region_find(get_system_memory(),
- addr, 1);
+ addr, size);
 
 if (!mrs.mr) {
 error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, 
addr);
@@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
 MemoryRegion *mr = NULL;
 void *ptr;
 
-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
@@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
 void *ptr;
 uint64_t physaddr;
 
-ptr = gpa2hva(, addr, _err);
+ptr = gpa2hva(, addr, 1, _err);
 if (local_err) {
 error_report_err(local_err);
 return;
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index dee3b45930..d145f916b3 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,24 @@
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
   'if': 'defined(TARGET_I386)' }
 
+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 5.1
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 27ebfa3ad2..42bcfe6dc0 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
 
 return data;
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
+error_setg(errp, "SEV inject secret failed");
+}
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index e5ee13309c..fed4588185 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
 {
 return NULL;
 }
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa)
+{
+return 1;
+}
diff --git a/target

SEV: QMP support for Inject-Launch-Secret

2020-06-30 Thread Tobin Feldman-Fitzthum
This is an update to part of a patch submitted previously to
provide support for injecting a secret blob into guest memory
using AMD SEV.

The user provides a header and a wrapped secret blob via QMP,
which are provided to the AMD Secure Processor and injected
into the guest.

Note that this patch requires the user to provide the guest
physical address where the secret will be injected via QMP.

Tobin Feldman-Fitzthum (1):
  sev: add sev-inject-launch-secret

 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 ++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  9 ++
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 66 +++
 target/i386/trace-events  |  1 +
 8 files changed, 108 insertions(+), 4 deletions(-)

-- 
2.20.1 (Apple Git-117)




[PATCH 0/2] Add support for SEV Launch Secret Injection

2020-05-28 Thread Tobin Feldman-Fitzthum
This patchset contains two patches. The first enables QEMU
to facilitate the injection of a secret blob into the guest
memory.

The second enables QEMU to parse the guest ROM to determine
the address at which the secret should be injected.

Tobin Feldman-Fitzthum (2):
  sev: add sev-inject-launch-secret
  sev: scan guest ROM for launch secret address

 include/sysemu/sev.h   |   2 +
 qapi/misc-target.json  |  20 +++
 target/i386/monitor.c  |   8 +++
 target/i386/sev-stub.c |   5 ++
 target/i386/sev.c  | 113 +
 target/i386/sev_i386.h |  16 ++
 target/i386/trace-events   |   1 +
 tests/qtest/qmp-cmd-test.c |   6 +-
 8 files changed, 168 insertions(+), 3 deletions(-)

-- 
2.20.1 (Apple Git-117)




[PATCH 1/2] sev: add sev-inject-launch-secret

2020-05-28 Thread Tobin Feldman-Fitzthum
From: Tobin Feldman-Fitzthum 

AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU faciliates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 include/sysemu/sev.h   |  2 +
 qapi/misc-target.json  | 20 +
 target/i386/monitor.c  |  8 
 target/i386/sev-stub.c |  5 +++
 target/i386/sev.c  | 83 ++
 target/i386/trace-events   |  1 +
 tests/qtest/qmp-cmd-test.c |  6 +--
 7 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..313ee30fc8 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@
 
 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+uint64_t gpa);
 #endif
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index dee3b45930..27458b765b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,26 @@
 { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
   'if': 'defined(TARGET_I386)' }
 
+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+GPA provided here will be ignored if guest ROM specifies 
+the a launch secret GPA.
+#
+# Since: 5.0.0
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet_hdr': 'str', 'secret': 'str', 'gpa': 'uint64' },
+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 27ebfa3ad2..5c2b7d2c17 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,11 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
 
 return data;
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+if (sev_inject_launch_secret(packet_hdr,secret,gpa) != 0)
+  error_setg(errp, "SEV inject secret failed");
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index e5ee13309c..2b8c5f1f53 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
 {
 return NULL;
 }
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+uint64_t gpa)
+{
+   return 1;
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 846018a12d..774e47d9d1 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -28,6 +28,7 @@
 #include "sysemu/runstate.h"
 #include "trace.h"
 #include "migration/blocker.h"
+#include "exec/address-spaces.h"
 
 #define DEFAULT_GUEST_POLICY0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE  "/dev/sev"
@@ -743,6 +744,88 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
 return 0;
 }
 
+
+static void *
+gpa2hva(hwaddr addr, uint64_t size)
+{
+MemoryRegionSection mrs = memory_region_find(get_system_memory(),
+ addr, size);
+
+if (!mrs.mr) {
+error_report("No memory is mapped at address 0x%" HWADDR_PRIx, addr);
+return NULL;
+}
+
+if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
+error_report("Memory at address 0x%" HWADDR_PRIx "is not RAM", addr);
+memory_region_unref(mrs.mr);
+return NULL;
+}
+
+return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
+}
+
+int sev_inject_launch_secret(const char *packet_hdr,
+ const char *secret, uint64_t gpa)
+{
+struct kvm_sev_launch_secret *input = NULL;
+guchar *data = NULL, *hdr = NULL;
+int error, ret = 1;
+void *hva;
+gsize hdr_sz = 0, data_sz = 0;
+
+/* secret can be inject only in this state */
+if (!sev_check_state(SEV_STATE_LAUNCH_SECRET)) {
+   error_report("Not in correct state. %x",sev_state->state);
+   return 1;
+}
+
+hdr = g_base64_decode(packet_hdr, _sz);
+if (!hdr || !hdr_sz) {
+error_report("SEV: Failed to decode sequence header");
+return 1;
+}
+
+data = g_base64_decode(secret, _sz);
+if (!data || !data_sz) {
+error_report("SEV: Failed to decode data");
+goto err;
+  

[PATCH 2/2] sev: scan guest ROM for launch secret address

2020-05-28 Thread Tobin Feldman-Fitzthum
From: Tobin Feldman-Fitzthum 

In addition to using QMP to provide the guest memory address
that the launch secret blob will be injected into, the
secret address can also be specified in the guest ROM. This
patch adds sev_find_secret_gpa, which scans the ROM page by
page to find a launch secret table identified by a GUID. If
the table is found, the address it contains will be used
in place of any address specified via QMP.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 target/i386/sev.c  | 34 --
 target/i386/sev_i386.h | 16 
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 774e47d9d1..4adc56d7e3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -706,6 +706,8 @@ sev_guest_init(const char *id)
 s->api_major = status.api_major;
 s->api_minor = status.api_minor;
 
+s->secret_gpa = 0;
+
 trace_kvm_sev_init();
 ret = sev_ioctl(s->sev_fd, KVM_SEV_INIT, NULL, _error);
 if (ret) {
@@ -731,6 +733,28 @@ err:
 return NULL;
 }
 
+static void
+sev_find_secret_gpa(uint8_t *ptr, uint64_t len)
+{
+uint64_t offset;
+
+SevROMSecretTable *secret_table;
+QemuUUID secret_table_guid;
+
+qemu_uuid_parse(SEV_ROM_SECRET_GUID,_table_guid);
+secret_table_guid = qemu_uuid_bswap(secret_table_guid);
+
+offset = len - 0x1000;
+while(offset > 0) {
+secret_table = (SevROMSecretTable *)(ptr + offset);
+if(qemu_uuid_is_equal(_table_guid, (QemuUUID *) secret_table)){
+sev_state->secret_gpa = (long unsigned int) secret_table->base;
+break;
+}
+offset -= 0x1000;
+}
+}
+
 int
 sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
 {
@@ -738,6 +762,9 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
 
 /* if SEV is in update state then encrypt the data else do nothing */
 if (sev_check_state(SEV_STATE_LAUNCH_UPDATE)) {
+if(!sev_state->secret_gpa) {
+sev_find_secret_gpa(ptr, len);
+   }
 return sev_launch_update_data(ptr, len);
 }
 
@@ -776,8 +803,8 @@ int sev_inject_launch_secret(const char *packet_hdr,
 
 /* secret can be inject only in this state */
 if (!sev_check_state(SEV_STATE_LAUNCH_SECRET)) {
-   error_report("Not in correct state. %x",sev_state->state);
-   return 1;
+error_report("Not in correct state. %x",sev_state->state);
+return 1;
 }
 
 hdr = g_base64_decode(packet_hdr, _sz);
@@ -792,6 +819,9 @@ int sev_inject_launch_secret(const char *packet_hdr,
 goto err;
 }
 
+if(sev_state->secret_gpa)
+gpa = sev_state->secret_gpa;
+
 hva = gpa2hva(gpa, data_sz);
 if (!hva) {
 goto err;
diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 8ada9d385d..b1f9ab93bb 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -19,6 +19,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sev.h"
 #include "qemu/error-report.h"
+#include "qemu/uuid.h"
 #include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG0x1
@@ -28,6 +29,8 @@
 #define SEV_POLICY_DOMAIN   0x10
 #define SEV_POLICY_SEV  0x20
 
+#define SEV_ROM_SECRET_GUID "adf956ad-e98c-484c-ae11-b51c7d336447"
+
 #define TYPE_QSEV_GUEST_INFO "sev-guest"
 #define QSEV_GUEST_INFO(obj)  \
 OBJECT_CHECK(QSevGuestInfo, (obj), TYPE_QSEV_GUEST_INFO)
@@ -42,6 +45,18 @@ extern SevCapability *sev_get_capabilities(void);
 
 typedef struct QSevGuestInfo QSevGuestInfo;
 typedef struct QSevGuestInfoClass QSevGuestInfoClass;
+typedef struct SevROMSecretTable SevROMSecretTable;
+
+/**
+ * If guest physical address for the launch secret is
+ * provided in the ROM, it should be in the following
+ * page-aligned structure.
+ */
+struct SevROMSecretTable {
+QemuUUID guid;
+unsigned int base;
+unsigned int size;
+};
 
 /**
  * QSevGuestInfo:
@@ -78,6 +93,7 @@ struct SEVState {
 uint32_t cbitpos;
 uint32_t reduced_phys_bits;
 uint32_t handle;
+uint64_t secret_gpa;
 int sev_fd;
 SevState state;
 gchar *measurement;
-- 
2.20.1 (Apple Git-117)