Add AMD Secure Nested Paging (SEV-SNP) support

Data structures like struct setup_data have been moved to a separate setup_data.h header which bootparam.h relies on. Add setup_data.h to the cp_portable() list and sync it along with the other header files.

Note that currently struct setup_data is stripped away as part of generating bootparam.h, but that handling is no currently needed for setup_data.h since it doesn't pull in many external headers/dependencies. However, QEMU currently redefines struct setup_data in hw/i386/x86.c, so that will need to be removed as part of any header update that pulls in the new setup_data.h to avoid build bisect breakage.

Signed-off-by Michael Roth <[email protected]>

(quoted diff) +1 -0

@@ -1842,6 +1842,17 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
          }
      }
 
+    if (kvm_enabled() && (new_block->flags & RAM_GUEST_MEMFD)) {

From: Xiaoyao Li <[email protected]>

Add a new member "guest_memfd" to memory backends. When it's set to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm guest_memfd will be allocated during RAMBlock allocation.

Memory backend's @guest_memfd is wired with @require_guest_memfd field of MachineState. It avoid looking up the machine in phymem.c.

MachineState::require_guest_memfd is supposed to be set by any VMs that requires KVM guest memfd as private memory, e.g., TDX VM.

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

---
Changes in v4:
- rename "require_guest_memfd" to "guest_memfd" in struct
HostMemoryBackend; (David Hildenbrand)

Signed-off-by Michael Roth <[email protected]>

---
 backends/hostmem-file.c  | 1 +
 backends/hostmem-memfd.c | 1 +
 backends/hostmem-ram.c   | 1 +
 backends/hostmem.c       | 1 +
 hw/core/machine.c        | 5 +++++
 include/hw/boards.h      | 2 ++
 include/sysemu/hostmem.h | 1 +
 7 files changed, 12 insertions(+)

backends/hostmem-file.c +1 -0

index ac3e433cbd..3c69db7946 100644
@@ -85,6 +85,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     ram_flags |= fb->readonly ? RAM_READONLY_FD : 0;
     ram_flags |= fb->rom == ON_OFF_AUTO_ON ? RAM_READONLY : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
     ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
     ram_flags |= RAM_NAMED_FILE;
     return memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,

backends/hostmem-memfd.c +1 -0

index 3923ea9364..745ead0034 100644
@@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     name = host_memory_backend_get_name(backend);
     ram_flags = backend->share ? RAM_SHARED : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
     return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
                                           backend->size, ram_flags, fd, 0, errp);
 }

backends/hostmem-ram.c +0 -0

index d121249f0f..f7d81af783 100644

Quoted 60 lines · Michael Roth

From: Xiaoyao Li <[email protected]>

Add a new member "guest_memfd" to memory backends. When it's set to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm guest_memfd will be allocated during RAMBlock allocation.

Memory backend's @guest_memfd is wired with @require_guest_memfd field of MachineState. It avoid looking up the machine in phymem.c.

MachineState::require_guest_memfd is supposed to be set by any VMs that requires KVM guest memfd as private memory, e.g., TDX VM.

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

---
Changes in v4:
- rename "require_guest_memfd" to "guest_memfd" in struct
HostMemoryBackend; (David Hildenbrand)

Signed-off-by Michael Roth <[email protected]>

---
  backends/hostmem-file.c  | 1 +
  backends/hostmem-memfd.c | 1 +
  backends/hostmem-ram.c   | 1 +
  backends/hostmem.c       | 1 +
  hw/core/machine.c        | 5 +++++
  include/hw/boards.h      | 2 ++
  include/sysemu/hostmem.h | 1 +
  7 files changed, 12 insertions(+)

backends/hostmem-file.c +1 -0

index ac3e433cbd..3c69db7946 100644
@@ -85,6 +85,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
      ram_flags |= fb->readonly ? RAM_READONLY_FD : 0;
      ram_flags |= fb->rom == ON_OFF_AUTO_ON ? RAM_READONLY : 0;
      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
      ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
      ram_flags |= RAM_NAMED_FILE;
      return memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,

backends/hostmem-memfd.c +1 -0

index 3923ea9364..745ead0034 100644
@@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
      name = host_memory_backend_get_name(backend);
      ram_flags = backend->share ? RAM_SHARED : 0;
      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
      return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
                                            backend->size, ram_flags, fd, 0, errp);
  }

backends/hostmem-ram.c +0 -0

index d121249f0f..f7d81af783 100644
@@ -30,6 +30,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
      name = host_memory_backend_get_name(backend);
      ram_flags = backend->share ? RAM_SHARED : 0;

backends/hostmem-ram.c ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) · line 41

Quoted 5 lines

backends/hostmem-ram.c +1 -0

      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
      return memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend),
                                                    name, backend->size,
                                                    ram_flags, errp);

These change look a bit confusing to me, as I don't see how gmemfd can be used with either file or ram typed memory backends..

When specified gmemfd=on with those, IIUC it'll allocate both the memory (ramblock->host) and gmemfd, but without using ->host. Meanwhile AFAIU the ramblock->host will start to conflict with gmemfd in the future when it might be able to be mapp-able (having valid ->host).

I have a local fix for this (and actually more than below.. but starting from it), I'm not sure whether I overlooked something, but from reading the cover letter it's only using memfd backend which makes perfect sense to me so far.

Quoted 61 lines · Peter Xu

Quoted 60 lines · Michael Roth

From: Xiaoyao Li <[email protected]>

Add a new member "guest_memfd" to memory backends. When it's set to true, it enables RAM_GUEST_MEMFD in ram_flags, thus private kvm guest_memfd will be allocated during RAMBlock allocation.

Memory backend's @guest_memfd is wired with @require_guest_memfd field of MachineState. It avoid looking up the machine in phymem.c.

MachineState::require_guest_memfd is supposed to be set by any VMs that requires KVM guest memfd as private memory, e.g., TDX VM.

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

---
Changes in v4:
- rename "require_guest_memfd" to "guest_memfd" in struct
HostMemoryBackend; (David Hildenbrand)

Signed-off-by Michael Roth <[email protected]>

---
  backends/hostmem-file.c  | 1 +
  backends/hostmem-memfd.c | 1 +
  backends/hostmem-ram.c   | 1 +
  backends/hostmem.c       | 1 +
  hw/core/machine.c        | 5 +++++
  include/hw/boards.h      | 2 ++
  include/sysemu/hostmem.h | 1 +
  7 files changed, 12 insertions(+)

backends/hostmem-file.c +1 -0

index ac3e433cbd..3c69db7946 100644
@@ -85,6 +85,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
      ram_flags |= fb->readonly ? RAM_READONLY_FD : 0;
      ram_flags |= fb->rom == ON_OFF_AUTO_ON ? RAM_READONLY : 0;
      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
      ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
      ram_flags |= RAM_NAMED_FILE;
      return memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,

backends/hostmem-memfd.c +1 -0

index 3923ea9364..745ead0034 100644
@@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
      name = host_memory_backend_get_name(backend);
      ram_flags = backend->share ? RAM_SHARED : 0;
      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
      return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
                                            backend->size, ram_flags, fd, 0, errp);
  }

backends/hostmem-ram.c +0 -0

index d121249f0f..f7d81af783 100644
@@ -30,6 +30,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
      name = host_memory_backend_get_name(backend);
      ram_flags = backend->share ? RAM_SHARED : 0;

Quoted 8 lines

backends/hostmem-ram.c ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) · line 41

Quoted 5 lines

backends/hostmem-ram.c +1 -0

      ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+    ram_flags |= backend->guest_memfd ? RAM_GUEST_MEMFD : 0;
      return memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend),
                                                    name, backend->size,
                                                    ram_flags, errp);

These change look a bit confusing to me, as I don't see how gmemfd can be used with either file or ram typed memory backends..

I recall that the following should work:

"private" memory will come from guest_memfd, "shared" (as in, accessible by the host) will come from anonymous memory.

This "anon" memory cannot be "shared" with other processes, but virtio-kernel etc. can just use it.

To "share" the memory with other processes, we'd need memfd/file.

Quoted 6 lines

When specified gmemfd=on with those, IIUC it'll allocate both the memory (ramblock->host) and gmemfd, but without using ->host. Meanwhile AFAIU the ramblock->host will start to conflict with gmemfd in the future when it might be able to be mapp-able (having valid ->host).

These will require a new guest_memfd memory backend (I recall that was discussed a couple of times).

Quoted 6 lines

I have a local fix for this (and actually more than below.. but starting from it), I'm not sure whether I overlooked something, but from reading the cover letter it's only using memfd backend which makes perfect sense to me so far.

Does the anon+guest_memfd combination not work or are you speculating about the usability (which I hopefully addressed above).

Quoted 4 lines

This "anon" memory cannot be "shared" with other processes, but virtio-kernel etc. can just use it.

To "share" the memory with other processes, we'd need memfd/file.

Ah OK, thanks David. Is this the planned long term solution for vhost-kernel?

Quoted 1 line

I wonder what happens if vhost tries to DMA to a region that is private

with this setup.

Quoted 1 line

AFAIU, it'll try to DMA to the fake address of ramblock->host that is

pointing to by the memory backend (either anon, shmem, file, etc.). The ideal case IIUC is it should crash QEMU because it's trying to access an illegal page which is private. But if with this model, it won't crash but silently populate some page in the non-gmemfd backend.

Is that expected?

Quoted 8 lines

Quoted 7 lines

When specified gmemfd=on with those, IIUC it'll allocate both the memory (ramblock->host) and gmemfd, but without using ->host. Meanwhile AFAIU the ramblock->host will start to conflict with gmemfd in the future when it might be able to be mapp-able (having valid ->host).

Quoted 2 lines

These will require a new guest_memfd memory backend (I recall that was discussed a couple of times).

Do you know if anyone is working on this one?

Quoted 8 lines

Quoted 7 lines

I have a local fix for this (and actually more than below.. but starting from it), I'm not sure whether I overlooked something, but from reading the cover letter it's only using memfd backend which makes perfect sense to me so far.

Quoted 2 lines

Does the anon+guest_memfd combination not work or are you speculating about the usability (which I hopefully addressed above).

IIUC, if with above solution and with how QEMU interacts memory convertions right now, at least hugetlb pages will suffer from double allocation, as kvm_convert_memory() won't free hugetlb pages even if converted to private.

It sounds like also doable (and also preferrable..) that for each of the VM we always stich with pages in the gmemfd page cache, no matter if it's shared or private. For private, we could zap all pgtables and sigbus any faults afterwards. I thought that was always the plan, but I could lose many latest informations..

So far my understanding is that Google that does shared+private guest_memfd kernel part won't be working on QEMU patches. I raised that to our management recently, that this would be a good project for RH to focus on.

I am not aware of real implementations of the guest_memfd backend (yet).

Quoted 4 lines · David Hildenbrand

So far my understanding is that Google that does shared+private guest_memfd kernel part won't be working on QEMU patches. I raised that to our management recently, that this would be a good project for RH to focus on.

Quoted 1 line

I am not aware of real implementations of the guest_memfd backend (yet).

I see, thanks, those are pretty useful information to me.

I think I have a rough picture now. Since I've already had some patches going that direction, I'll give it a shot. I'll keep you updated if I get something.

From: Xiaoyao Li <[email protected]>

Introduce the helper functions to set the attributes of a range of memory to private or shared.

This is necessary to notify KVM the private/shared attribute of each gpa range. KVM needs the information to decide the GPA needs to be mapped at hva-based shared memory or guest_memfd based private memory.

Signed-off-by Xiaoyao Li <[email protected]>

---
Changes in v4:
- move the check of kvm_supported_memory_attributes to the common kvm_set_memory_attributes(); (Wang Wei)
- change warn_report() to error_report() in kvm_set_memory_attributes() and drop the __func__; (Daniel)

Signed-off-by Michael Roth <[email protected]>

---
  accel/kvm/kvm-all.c  | 44 ++++++++++++++++++++++++++++++++++++++++++++
  include/sysemu/kvm.h |  3 +++
  2 files changed, 47 insertions(+)

accel/kvm/kvm-all.c +7 -0

index e83429b31e..df7a32735a 100644
@@ -92,6 +92,7 @@ static bool kvm_has_guest_debug;
  static int kvm_sstep_flags;
  static bool kvm_immediate_exit;
  static bool kvm_guest_memfd_supported;
+static uint64_t kvm_supported_memory_attributes;
  static hwaddr kvm_max_slot_size = ~0;
 
  static const KVMCapabilityInfo kvm_required_capabilites[] = {

@@ -1304,6 +1305,46 @@ void kvm_set_max_memslot_size(hwaddr max_slot_size)
      kvm_max_slot_size = max_slot_size;
  }
 
+static int kvm_set_memory_attributes(hwaddr start, hwaddr size, uint64_t attr)
+{
+    struct kvm_memory_attributes attrs;
+    int r;
+
+    if (kvm_supported_memory_attributes == 0) {

accel/kvm/kvm-all.c +26 -0

+    attrs.attributes = attr;
+    attrs.address = start;
+    attrs.size = size;
+    attrs.flags = 0;
+
+    r = kvm_vm_ioctl(kvm_state, KVM_SET_MEMORY_ATTRIBUTES, &attrs);
+    if (r) {
+        error_report("failed to set memory (0x%lx+%#zx) with attr 0x%lx error '%s'",
+                     start, size, attr, strerror(errno));
+    }
+    return r;
+}
+
+int kvm_set_memory_attributes_private(hwaddr start, hwaddr size)
+{
+    return kvm_set_memory_attributes(start, size, KVM_MEMORY_ATTRIBUTE_PRIVATE);
+}
+
+int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size)
+{
+    return kvm_set_memory_attributes(start, size, 0);
+}
+
  /* Called with KVMMemoryListener.slots_lock held */
  static void kvm_set_phys_mem(KVMMemoryListener *kml,
                               MemoryRegionSection *section, bool add)

@@ -2439,6 +2480,9 @@ static int kvm_init(MachineState *ms)
 
      kvm_guest_memfd_supported = kvm_check_extension(s, KVM_CAP_GUEST_MEMFD);
 
+    ret = kvm_check_extension(s, KVM_CAP_MEMORY_ATTRIBUTES);
+    kvm_supported_memory_attributes = ret > 0 ? ret : 0;
+
      if (object_property_find(OBJECT(current_machine), "kvm-type")) {
          g_autofree char *kvm_type = object_property_get_str(OBJECT(current_machine),
                                                              "kvm-type",

include/sysemu/kvm.h +0 -0

index b4913281e2..2cb3192509 100644

From: Xiaoyao Li <[email protected]>

When memory page is converted from private to shared, the original private memory is back'ed by guest_memfd. Introduce ram_block_discard_guest_memfd_range() for discarding memory in guest_memfd.

Originally-from: Isaku Yamahata <[email protected]>

Codeveloped-by Xiaoyao Li <[email protected]>

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

---
Changes in v5:
- Collect Reviewed-by from David;

Changes in in v4:
- Drop ram_block_convert_range() and open code its implementation in the next Patch.

Signed-off-by Michael Roth <[email protected]>

Quoted 7 lines · Michael Roth

From: Xiaoyao Li <[email protected]>

When memory page is converted from private to shared, the original private memory is back'ed by guest_memfd. Introduce ram_block_discard_guest_memfd_range() for discarding memory in guest_memfd.

Quoted 2 lines

Originally-from: Isaku Yamahata <[email protected]>

Codeveloped-by Xiaoyao Li <[email protected]>

"Co-developed-by"

Quoted 2 lines

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

Your SOB should go here.

Quoted 4 lines

---
Changes in v5:
- Collect Reviewed-by from David;

Quoted 5 lines

Changes in in v4:
- Drop ram_block_convert_range() and open code its implementation in the next Patch.

Signed-off-by Michael Roth <[email protected]>

I only received 3 patches from this series, and now I am confused: changelog talks about v5 and this is "PATCH v3"

Please make sure to send at least the cover letter along (I might not need the other 46 patches :D ).

Quoted 8 lines · David Hildenbrand

Quoted 7 lines · Michael Roth

From: Xiaoyao Li <[email protected]>

When memory page is converted from private to shared, the original private memory is back'ed by guest_memfd. Introduce ram_block_discard_guest_memfd_range() for discarding memory in guest_memfd.

Quoted 4 lines

Quoted 2 lines

Originally-from: Isaku Yamahata <[email protected]>

Codeveloped-by Xiaoyao Li <[email protected]>

"Co-developed-by"

Michael is using the patch from my TDX-QEMU v5 series[1]. I need to fix it.

Quoted 13 lines

Quoted 2 lines

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

Your SOB should go here.

Quoted 8 lines

---
Changes in v5:
- Collect Reviewed-by from David;

Changes in in v4:
- Drop ram_block_convert_range() and open code its implementation in the next Patch.

Quoted 4 lines

Quoted 1 line

Signed-off-by Michael Roth <[email protected]>

I only received 3 patches from this series, and now I am confused:
changelog talks about v5 and this is "PATCH v3"

As above, because the guest_memfd patches in my TDX-QEMU v5[1] were directly picked for this series, so the change history says v5. They are needed by SEV-SNP as well.

Quoted 9 lines · Michael Roth

From: Xiaoyao Li <[email protected]>

When memory page is converted from private to shared, the original private memory is back'ed by guest_memfd. Introduce ram_block_discard_guest_memfd_range() for discarding memory in guest_memfd.

Originally-from: Isaku Yamahata <[email protected]>

Codeveloped-by Xiaoyao Li <[email protected]>

"Co-developed-by"

Quoted 2 lines

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

Your SOB should go here.

Quoted 8 lines

---
Changes in v5:
- Collect Reviewed-by from David;

Changes in in v4:
- Drop ram_block_convert_range() and open code its implementation in the next Patch.

Quoted 1 line

Signed-off-by Michael Roth <[email protected]>

I only received 3 patches from this series, and now I am confused: changelog talks about v5 and this is "PATCH v3"

Please make sure to send at least the cover letter along (I might not need the other 46 patches :D ).

Signature · 4 lines

-- 
Cheers,

David / dhildenb

Quoted 31 lines · David Hildenbrand

Quoted 9 lines · Michael Roth

From: Xiaoyao Li <[email protected]>

When memory page is converted from private to shared, the original private memory is back'ed by guest_memfd. Introduce ram_block_discard_guest_memfd_range() for discarding memory in guest_memfd.

Originally-from: Isaku Yamahata <[email protected]>

Codeveloped-by Xiaoyao Li <[email protected]>

"Co-developed-by"

Quoted 2 lines

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

Your SOB should go here.

Quoted 9 lines

---
Changes in v5:
- Collect Reviewed-by from David;

Changes in in v4:
- Drop ram_block_convert_range() and open code its implementation in the next Patch.

Signed-off-by Michael Roth <[email protected]>

I only received 3 patches from this series, and now I am confused: changelog talks about v5 and this is "PATCH v3"

Quoted 2 lines

Please make sure to send at least the cover letter along (I might not need the other 46 patches :D ).

Sorry for the confusion, you got auto-Cc'd by git, which is good, but not sure there's a good way to make sure everyone gets a copy of the cover letter. I could see how it would help useful to potential reviewers though. I'll try to come up with a script for it and take that approach in the future.

Quoted 32 lines · Michael Roth

Quoted 31 lines · David Hildenbrand

Quoted 9 lines · Michael Roth

From: Xiaoyao Li <[email protected]>

When memory page is converted from private to shared, the original private memory is back'ed by guest_memfd. Introduce ram_block_discard_guest_memfd_range() for discarding memory in guest_memfd.

Originally-from: Isaku Yamahata <[email protected]>

Codeveloped-by Xiaoyao Li <[email protected]>

"Co-developed-by"

Quoted 2 lines

Signed-off-by Xiaoyao Li <[email protected]>

Reviewed-by David Hildenbrand <[email protected]>

Your SOB should go here.

Quoted 9 lines

---
Changes in v5:
- Collect Reviewed-by from David;

Changes in in v4:
- Drop ram_block_convert_range() and open code its implementation in the next Patch.

Signed-off-by Michael Roth <[email protected]>

I only received 3 patches from this series, and now I am confused: changelog talks about v5 and this is "PATCH v3"

Quoted 8 lines

Quoted 2 lines

Please make sure to send at least the cover letter along (I might not need the other 46 patches :D ).

Sorry for the confusion, you got auto-Cc'd by git, which is good, but not sure there's a good way to make sure everyone gets a copy of the cover letter. I could see how it would help useful to potential reviewers though. I'll try to come up with a script for it and take that approach in the future.

A script shared with me in the past to achieve that in most cases:

$ cat cc-cmd.sh
#!/bin/bash

if [[ $1 == *gitsendemail.msg* || $1 == *cover-letter* ]]; then

        grep ': .* <.*@.*>' -h *.patch | sed 's/^.*: //' | sort | uniq

fi

And attach to "git send-email ... *.patch": --cc-cmd=./cc-cmd.sh

Signature · 4 lines

-- 
Cheers,

David / dhildenb

TODO: squash into "kvm: handle KVM_EXIT_MEMORY_FAULT"

qemu_host_page_size has been superseded by qemu_real_host_page_size() in newer QEMU, so update the patch accordingly.

Signed-off-by Michael Roth <[email protected]>

---
  accel/kvm/kvm-all.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

accel/kvm/kvm-all.c +3 -3

index 2fdc07a472..a9c19ab9a1 100644
@@ -2912,8 +2912,8 @@ static int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
      void *addr;
      int ret = -1;
 
-    if (!QEMU_PTR_IS_ALIGNED(start, qemu_host_page_size) ||
-        !QEMU_PTR_IS_ALIGNED(size, qemu_host_page_size)) {
+    if (!QEMU_PTR_IS_ALIGNED(start, qemu_real_host_page_size()) ||
+        !QEMU_PTR_IS_ALIGNED(size, qemu_real_host_page_size())) {
          return -1;
      }
 

@@ -2943,7 +2943,7 @@ static int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
          rb = qemu_ram_block_from_host(addr, false, &offset);
 
          if (to_private) {
-            if (rb->page_size != qemu_host_page_size) {
+            if (rb->page_size != qemu_real_host_page_size()) {
                  /*
                  * shared memory is back'ed by  hugetlb, which is supposed to be
                  * pre-allocated and doesn't need to be discarded

Some subsystems like VFIO might disable ram block discard for uncoordinated cases. Since kvm_convert_memory()/guest_memfd don't implement a RamDiscardManager handler to convey discard operations to various listeners like VFIO. > Because of this, sequences like the following can result due to stale IOMMU mappings:

- convert page shared->private
- discard shared page
- convert page private->shared
- new page is allocated
- issue DMA operations against that shared page

Address this by taking ram_block_discard_is_enabled() into account when deciding whether or not to discard pages.

Signed-off-by Michael Roth <[email protected]>

---
  accel/kvm/kvm-all.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

accel/kvm/kvm-all.c +6 -2

index 53ce4f091e..6ae03c880f 100644
@@ -2962,10 +2962,14 @@ static int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
                  */
                  return 0;
              } else {
-                ret = ram_block_discard_range(rb, offset, size);
+                ret = ram_block_discard_is_disabled()
+                      ? ram_block_discard_range(rb, offset, size)
+                      : 0;
              }
          } else {
-            ret = ram_block_discard_guest_memfd_range(rb, offset, size);
+            ret = ram_block_discard_is_disabled()
+                  ? ram_block_discard_guest_memfd_range(rb, offset, size)
+                  : 0;
          }
      } else {
          error_report("Convert non guest_memfd backed memory region "

Quoted 5 lines · Michael Roth

Some subsystems like VFIO might disable ram block discard for uncoordinated cases. Since kvm_convert_memory()/guest_memfd don't implement a RamDiscardManager handler to convey discard operations to various listeners like VFIO. > Because of this, sequences like the following can result due to stale IOMMU mappings:

Alternatively, should guest-memfd memory regions call ram_block_discard_require(true)? This will prevent VFIO from operating, but it will avoid consuming twice the memory.

If desirable, guest-memfd support can be changed to implement an extension of RamDiscardManager that notifies about private/shared memory changes, and then guest-memfd would be able to support coordinated discard. But I wonder

if that's doable at all - how common are shared<->private flips, and is it feasible to change the IOMMU page tables every time?

If the real solution is SEV-TIO (which means essentially guest_memfd support for VFIO), calling ram_block_discard_require(true) may be the simplest stopgap solution.

Paolo

Quoted 35 lines

- convert page shared->private
- discard shared page
- convert page private->shared
- new page is allocated
- issue DMA operations against that shared page

Address this by taking ram_block_discard_is_enabled() into account when deciding whether or not to discard pages.

Signed-off-by Michael Roth <[email protected]>

---
  accel/kvm/kvm-all.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

accel/kvm/kvm-all.c +6 -2

index 53ce4f091e..6ae03c880f 100644
@@ -2962,10 +2962,14 @@ static int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
                  */
                  return 0;
              } else {
-                ret = ram_block_discard_range(rb, offset, size);
+                ret = ram_block_discard_is_disabled()
+                      ? ram_block_discard_range(rb, offset, size)
+                      : 0;
              }
          } else {
-            ret = ram_block_discard_guest_memfd_range(rb, offset, size);
+            ret = ram_block_discard_is_disabled()
+                  ? ram_block_discard_guest_memfd_range(rb, offset, size)
+                  : 0;
          }
      } else {
          error_report("Convert non guest_memfd backed memory region "

Currently all SEV/SEV-ES functionality is managed through a single 'sev-guest' QOM type. With upcoming support for SEV-SNP, taking this same approach won't work well since some of the properties/state managed by 'sev-guest' is not applicable to SEV-SNP, which will instead rely on a new QOM type with its own set of properties/state.

To prepare for this, this patch moves common state into an abstract 'sev-common' parent type to encapsulate properties/state that are common to both SEV/SEV-ES and SEV-SNP, leaving only SEV/SEV-ES-specific properties/state in the current 'sev-guest' type. This should not affect current behavior or command-line options.

As part of this patch, some related changes are also made:

- a static 'sev_guest' variable is currently used to keep track of the 'sev-guest' instance. SEV-SNP would similarly introduce an 'sev_snp_guest' static variable. But these instances are now available via qdev_get_machine()->cgs, so switch to using that instead and drop the static variable.

- 'sev_guest' is currently used as the name for the static variable holding a pointer to the 'sev-guest' instance. Re-purpose the name as a local variable referring the 'sev-guest' instance, and use that consistently throughout the code so it can be easily distinguished from sev-common/sev-snp-guest instances.

- 'sev' is generally used as the name for local variables holding a pointer to the 'sev-guest' instance. In cases where that now points to common state, use the name 'sev_common'; in cases where that now points to state specific to 'sev-guest' instance, use the name 'sev_guest'

Signed-off-by Michael Roth <[email protected]>

---
 qapi/qom.json     |  32 ++--
 target/i386/sev.c | 457 ++++++++++++++++++++++++++--------------------
 target/i386/sev.h |   3 +
 3 files changed, 281 insertions(+), 211 deletions(-)

qapi/qom.json +1 -2

index baae3a183f..66b5781ca6 100644
@@ -875,12 +875,29 @@
   'data': { '*filename': 'str' } }
 
 ##
-# @SevGuestProperties:
+# @SevCommonProperties:
 #
-# Properties for sev-guest objects.

qapi/qom.json +13 -9

+##
+{ 'struct': 'SevCommonProperties',
+  'data': { '*sev-device': 'str',
+            '*cbitpos': 'uint32',
+            'reduced-phys-bits': 'uint32' } }
+
+##
+# @SevGuestProperties:
+#
+# Properties for sev-guest objects.
+#
 # @dh-cert-file: guest owners DH certificate (encoded with base64)
 #
 # @session-file: guest owners session parameters (encoded with base64)

@@ -889,11 +906,6 @@
 #
 # @handle: SEV firmware handle (default: 0)
 #
-# @cbitpos: C-bit location in page table entry (default: 0)
-#
-# @reduced-phys-bits: number of bits in physical addresses that become
-#     unavailable when SEV is enabled
-#
 # @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a
 #     designated guest firmware page for measured boot with -kernel
 #     (default: false) (since 6.2)

@@ -901,13 +913,11 @@
 # Since: 2.12
 ##
 { 'struct': 'SevGuestProperties',
-  'data': { '*sev-device': 'str',
-            '*dh-cert-file': 'str',
+  'base': 'SevCommonProperties',
+  'data': { '*dh-cert-file': 'str',
             '*session-file': 'str',
             '*policy': 'uint32',
             '*handle': 'uint32',
-            '*cbitpos': 'uint32',
-            'reduced-phys-bits': 'uint32',
             '*kernel-hashes': 'bool' } }
 
 ##

target/i386/sev.c +49 -104

index 9dab4060b8..63a220de5e 100644
@@ -40,48 +40,53 @@
 #include "hw/i386/pc.h"
 #include "exec/address-spaces.h"
 
-#define TYPE_SEV_GUEST "sev-guest"
+OBJECT_DECLARE_SIMPLE_TYPE(SevCommonState, SEV_COMMON)
 OBJECT_DECLARE_SIMPLE_TYPE(SevGuestState, SEV_GUEST)
 
-
-/**
- * SevGuestState:
- *
- * The SevGuestState object is used for creating and managing a SEV
- * guest.
- *
- * # $QEMU \
- *         -object sev-guest,id=sev0 \
- *         -machine ...,memory-encryption=sev0
- */
-struct SevGuestState {
+struct SevCommonState {
     X86ConfidentialGuest parent_obj;
 
     int kvm_type;
 
     /* configuration parameters */
     char *sev_device;
-    uint32_t policy;
-    char *dh_cert_file;
-    char *session_file;
     uint32_t cbitpos;
     uint32_t reduced_phys_bits;
-    bool kernel_hashes;
 
     /* runtime state */
-    uint32_t handle;
     uint8_t api_major;
     uint8_t api_minor;
     uint8_t build_id;
     int sev_fd;
     SevState state;
-    gchar *measurement;
 
     uint32_t reset_cs;
     uint32_t reset_ip;
     bool reset_data_valid;
 };
 
+/**
+ * SevGuestState:
+ *
+ * The SevGuestState object is used for creating and managing a SEV
+ * guest.
+ *
+ * # $QEMU \
+ *         -object sev-guest,id=sev0 \
+ *         -machine ...,memory-encryption=sev0
+ */
+struct SevGuestState {
+    SevCommonState sev_common;
+    gchar *measurement;
+
+    /* configuration parameters */
+    uint32_t handle;
+    uint32_t policy;
+    char *dh_cert_file;
+    char *session_file;
+    bool kernel_hashes;
+};
+
 #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE      "/dev/sev"
 

@@ -127,7 +132,6 @@ typedef struct QEMU_PACKED PaddedSevHashTable {
 
 QEMU_BUILD_BUG_ON(sizeof(PaddedSevHashTable) % 16 != 0);
 
-static SevGuestState *sev_guest;
 static Error *sev_mig_blocker;
 
 static const char *const sev_fw_errlist[] = {

@@ -208,21 +212,21 @@ fw_error_to_str(int code)
 }
 
 static bool
-sev_check_state(const SevGuestState *sev, SevState state)
+sev_check_state(const SevCommonState *sev_common, SevState state)
 {
-    assert(sev);
-    return sev->state == state ? true : false;
+    assert(sev_common);
+    return sev_common->state == state ? true : false;
 }
 
 static void
-sev_set_guest_state(SevGuestState *sev, SevState new_state)
+sev_set_guest_state(SevCommonState *sev_common, SevState new_state)
 {
     assert(new_state < SEV_STATE__MAX);
-    assert(sev);
+    assert(sev_common);
 
-    trace_kvm_sev_change_state(SevState_str(sev->state),
+    trace_kvm_sev_change_state(SevState_str(sev_common->state),
                                SevState_str(new_state));
-    sev->state = new_state;
+    sev_common->state = new_state;
 }
 
 static void

@@ -289,111 +293,61 @@ static struct RAMBlockNotifier sev_ram_notifier = {
     .ram_block_removed = sev_ram_block_removed,
 };
 
-static void
-sev_guest_finalize(Object *obj)
-{
-}
-
-static char *
-sev_guest_get_session_file(Object *obj, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    return s->session_file ? g_strdup(s->session_file) : NULL;
-}
-
-static void
-sev_guest_set_session_file(Object *obj, const char *value, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    s->session_file = g_strdup(value);
-}
-
-static char *
-sev_guest_get_dh_cert_file(Object *obj, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    return g_strdup(s->dh_cert_file);
-}
-
-static void
-sev_guest_set_dh_cert_file(Object *obj, const char *value, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    s->dh_cert_file = g_strdup(value);
-}
-
-static char *
-sev_guest_get_sev_device(Object *obj, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    return g_strdup(sev->sev_device);
-}
-
-static void
-sev_guest_set_sev_device(Object *obj, const char *value, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    sev->sev_device = g_strdup(value);
-}
-
-static bool sev_guest_get_kernel_hashes(Object *obj, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    return sev->kernel_hashes;
-}
-
-static void sev_guest_set_kernel_hashes(Object *obj, bool value, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    sev->kernel_hashes = value;
-}
-
 bool
 sev_enabled(void)
 {
-    return !!sev_guest;
+    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
+
+    return !!object_dynamic_cast(OBJECT(cgs), TYPE_SEV_COMMON);
 }
 
 bool
 sev_es_enabled(void)
 {
-    return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
+    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
+
+    return sev_enabled() && (SEV_GUEST(cgs)->policy & SEV_POLICY_ES);
 }
 
 uint32_t
 sev_get_cbit_position(void)
 {
-    return sev_guest ? sev_guest->cbitpos : 0;
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+
+    return sev_common ? sev_common->cbitpos : 0;
 }
 
 uint32_t
 sev_get_reduced_phys_bits(void)
 {
-    return sev_guest ? sev_guest->reduced_phys_bits : 0;
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+
+    return sev_common ? sev_common->reduced_phys_bits : 0;
 }
 
 static SevInfo *sev_get_info(void)
 {
     SevInfo *info;
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+    SevGuestState *sev_guest =
+        (SevGuestState *)object_dynamic_cast(OBJECT(sev_common),
+                                             TYPE_SEV_GUEST);
 
     info = g_new0(SevInfo, 1);
     info->enabled = sev_enabled();
 
     if (info->enabled) {
-        info->api_major = sev_guest->api_major;
-        info->api_minor = sev_guest->api_minor;
-        info->build_id = sev_guest->build_id;
-        info->policy = sev_guest->policy;
-        info->state = sev_guest->state;
-        info->handle = sev_guest->handle;
+        if (sev_guest) {
+            info->handle = sev_guest->handle;

target/i386/sev.c +8 -2

     g_autofree guchar *data = NULL;
     g_autofree guchar *buf = NULL;
     gsize len;

@@ -602,8 +567,10 @@ static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
         return NULL;
     }
 
+    sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+
     /* Query the report length */
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
+    ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
             &input, &err);
     if (ret < 0) {
         if (err != SEV_RET_INVALID_LEN) {

@@ -619,7 +586,7 @@ static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
     memcpy(input.mnonce, buf, sizeof(input.mnonce));
 
     /* Query the report */
-    ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
+    ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
             &input, &err);
     if (ret) {
         error_setg_errno(errp, errno, "SEV: Failed to get attestation report"
 
+
+/* sev guest info common to sev/sev-es/sev-snp */
+static const TypeInfo sev_common_info = {
+    .parent = TYPE_X86_CONFIDENTIAL_GUEST,

Quoted 51 lines · Michael Roth

Currently all SEV/SEV-ES functionality is managed through a single 'sev-guest' QOM type. With upcoming support for SEV-SNP, taking this same approach won't work well since some of the properties/state managed by 'sev-guest' is not applicable to SEV-SNP, which will instead rely on a new QOM type with its own set of properties/state.

To prepare for this, this patch moves common state into an abstract 'sev-common' parent type to encapsulate properties/state that are common to both SEV/SEV-ES and SEV-SNP, leaving only SEV/SEV-ES-specific properties/state in the current 'sev-guest' type. This should not affect current behavior or command-line options.

As part of this patch, some related changes are also made:

- a static 'sev_guest' variable is currently used to keep track of the 'sev-guest' instance. SEV-SNP would similarly introduce an 'sev_snp_guest' static variable. But these instances are now available via qdev_get_machine()->cgs, so switch to using that instead and drop the static variable.

- 'sev_guest' is currently used as the name for the static variable holding a pointer to the 'sev-guest' instance. Re-purpose the name as a local variable referring the 'sev-guest' instance, and use that consistently throughout the code so it can be easily distinguished from sev-common/sev-snp-guest instances.

- 'sev' is generally used as the name for local variables holding a pointer to the 'sev-guest' instance. In cases where that now points to common state, use the name 'sev_common'; in cases where that now points to state specific to 'sev-guest' instance, use the name 'sev_guest'

Signed-off-by Michael Roth <[email protected]>

---
 qapi/qom.json     |  32 ++--
 target/i386/sev.c | 457 ++++++++++++++++++++++++++--------------------
 target/i386/sev.h |   3 +
 3 files changed, 281 insertions(+), 211 deletions(-)

qapi/qom.json +1 -2

index baae3a183f..66b5781ca6 100644
@@ -875,12 +875,29 @@
   'data': { '*filename': 'str' } }
 
 ##
-# @SevGuestProperties:
+# @SevCommonProperties:
 #
-# Properties for sev-guest objects.

qapi/qom.json line 889

Quoted 10 lines

qapi/qom.json +7 -0

+# Properties common to objects that are derivatives of sev-common.
 #
 # @sev-device: SEV device to use (default: "/dev/sev")
 #
+# @cbitpos: C-bit location in page table entry (default: 0)
+#
+# @reduced-phys-bits: number of bits in physical addresses that become
+#     unavailable when SEV is enabled
+#
+# Since: 2.12

Not quite sure what we've done in this scenario before. It feels wierd to use '2.12' for the new base type, even though in effect the properties all existed since 2.12 in the sub-class.

Perhaps 'Since: 9.1' for the type, but 'Since: 2.12' for the properties, along with an explanatory comment about stuff moving into the new base type ?

Markus, opinions ?

Quoted 285 lines

qapi/qom.json +13 -9

+##
+{ 'struct': 'SevCommonProperties',
+  'data': { '*sev-device': 'str',
+            '*cbitpos': 'uint32',
+            'reduced-phys-bits': 'uint32' } }
+
+##
+# @SevGuestProperties:
+#
+# Properties for sev-guest objects.
+#
 # @dh-cert-file: guest owners DH certificate (encoded with base64)
 #
 # @session-file: guest owners session parameters (encoded with base64)

@@ -889,11 +906,6 @@
 #
 # @handle: SEV firmware handle (default: 0)
 #
-# @cbitpos: C-bit location in page table entry (default: 0)
-#
-# @reduced-phys-bits: number of bits in physical addresses that become
-#     unavailable when SEV is enabled
-#
 # @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a
 #     designated guest firmware page for measured boot with -kernel
 #     (default: false) (since 6.2)

@@ -901,13 +913,11 @@
 # Since: 2.12
 ##
 { 'struct': 'SevGuestProperties',
-  'data': { '*sev-device': 'str',
-            '*dh-cert-file': 'str',
+  'base': 'SevCommonProperties',
+  'data': { '*dh-cert-file': 'str',
             '*session-file': 'str',
             '*policy': 'uint32',
             '*handle': 'uint32',
-            '*cbitpos': 'uint32',
-            'reduced-phys-bits': 'uint32',
             '*kernel-hashes': 'bool' } }
 
 ##

target/i386/sev.c +49 -104

index 9dab4060b8..63a220de5e 100644
@@ -40,48 +40,53 @@
 #include "hw/i386/pc.h"
 #include "exec/address-spaces.h"
 
-#define TYPE_SEV_GUEST "sev-guest"
+OBJECT_DECLARE_SIMPLE_TYPE(SevCommonState, SEV_COMMON)
 OBJECT_DECLARE_SIMPLE_TYPE(SevGuestState, SEV_GUEST)
 
-
-/**
- * SevGuestState:
- *
- * The SevGuestState object is used for creating and managing a SEV
- * guest.
- *
- * # $QEMU \
- *         -object sev-guest,id=sev0 \
- *         -machine ...,memory-encryption=sev0
- */
-struct SevGuestState {
+struct SevCommonState {
     X86ConfidentialGuest parent_obj;
 
     int kvm_type;
 
     /* configuration parameters */
     char *sev_device;
-    uint32_t policy;
-    char *dh_cert_file;
-    char *session_file;
     uint32_t cbitpos;
     uint32_t reduced_phys_bits;
-    bool kernel_hashes;
 
     /* runtime state */
-    uint32_t handle;
     uint8_t api_major;
     uint8_t api_minor;
     uint8_t build_id;
     int sev_fd;
     SevState state;
-    gchar *measurement;
 
     uint32_t reset_cs;
     uint32_t reset_ip;
     bool reset_data_valid;
 };
 
+/**
+ * SevGuestState:
+ *
+ * The SevGuestState object is used for creating and managing a SEV
+ * guest.
+ *
+ * # $QEMU \
+ *         -object sev-guest,id=sev0 \
+ *         -machine ...,memory-encryption=sev0
+ */
+struct SevGuestState {
+    SevCommonState sev_common;
+    gchar *measurement;
+
+    /* configuration parameters */
+    uint32_t handle;
+    uint32_t policy;
+    char *dh_cert_file;
+    char *session_file;
+    bool kernel_hashes;
+};
+
 #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE      "/dev/sev"
 

@@ -127,7 +132,6 @@ typedef struct QEMU_PACKED PaddedSevHashTable {
 
 QEMU_BUILD_BUG_ON(sizeof(PaddedSevHashTable) % 16 != 0);
 
-static SevGuestState *sev_guest;
 static Error *sev_mig_blocker;
 
 static const char *const sev_fw_errlist[] = {

@@ -208,21 +212,21 @@ fw_error_to_str(int code)
 }
 
 static bool
-sev_check_state(const SevGuestState *sev, SevState state)
+sev_check_state(const SevCommonState *sev_common, SevState state)
 {
-    assert(sev);
-    return sev->state == state ? true : false;
+    assert(sev_common);
+    return sev_common->state == state ? true : false;
 }
 
 static void
-sev_set_guest_state(SevGuestState *sev, SevState new_state)
+sev_set_guest_state(SevCommonState *sev_common, SevState new_state)
 {
     assert(new_state < SEV_STATE__MAX);
-    assert(sev);
+    assert(sev_common);
 
-    trace_kvm_sev_change_state(SevState_str(sev->state),
+    trace_kvm_sev_change_state(SevState_str(sev_common->state),
                                SevState_str(new_state));
-    sev->state = new_state;
+    sev_common->state = new_state;
 }
 
 static void

@@ -289,111 +293,61 @@ static struct RAMBlockNotifier sev_ram_notifier = {
     .ram_block_removed = sev_ram_block_removed,
 };
 
-static void
-sev_guest_finalize(Object *obj)
-{
-}
-
-static char *
-sev_guest_get_session_file(Object *obj, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    return s->session_file ? g_strdup(s->session_file) : NULL;
-}
-
-static void
-sev_guest_set_session_file(Object *obj, const char *value, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    s->session_file = g_strdup(value);
-}
-
-static char *
-sev_guest_get_dh_cert_file(Object *obj, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    return g_strdup(s->dh_cert_file);
-}
-
-static void
-sev_guest_set_dh_cert_file(Object *obj, const char *value, Error **errp)
-{
-    SevGuestState *s = SEV_GUEST(obj);
-
-    s->dh_cert_file = g_strdup(value);
-}
-
-static char *
-sev_guest_get_sev_device(Object *obj, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    return g_strdup(sev->sev_device);
-}
-
-static void
-sev_guest_set_sev_device(Object *obj, const char *value, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    sev->sev_device = g_strdup(value);
-}
-
-static bool sev_guest_get_kernel_hashes(Object *obj, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    return sev->kernel_hashes;
-}
-
-static void sev_guest_set_kernel_hashes(Object *obj, bool value, Error **errp)
-{
-    SevGuestState *sev = SEV_GUEST(obj);
-
-    sev->kernel_hashes = value;
-}
-
 bool
 sev_enabled(void)
 {
-    return !!sev_guest;
+    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
+
+    return !!object_dynamic_cast(OBJECT(cgs), TYPE_SEV_COMMON);
 }
 
 bool
 sev_es_enabled(void)
 {
-    return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
+    ConfidentialGuestSupport *cgs = MACHINE(qdev_get_machine())->cgs;
+
+    return sev_enabled() && (SEV_GUEST(cgs)->policy & SEV_POLICY_ES);
 }
 
 uint32_t
 sev_get_cbit_position(void)
 {
-    return sev_guest ? sev_guest->cbitpos : 0;
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+
+    return sev_common ? sev_common->cbitpos : 0;
 }
 
 uint32_t
 sev_get_reduced_phys_bits(void)
 {
-    return sev_guest ? sev_guest->reduced_phys_bits : 0;
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+
+    return sev_common ? sev_common->reduced_phys_bits : 0;
 }
 
 static SevInfo *sev_get_info(void)
 {
     SevInfo *info;
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+    SevGuestState *sev_guest =
+        (SevGuestState *)object_dynamic_cast(OBJECT(sev_common),
+                                             TYPE_SEV_GUEST);
 
     info = g_new0(SevInfo, 1);
     info->enabled = sev_enabled();
 
     if (info->enabled) {
-        info->api_major = sev_guest->api_major;
-        info->api_minor = sev_guest->api_minor;
-        info->build_id = sev_guest->build_id;
-        info->policy = sev_guest->policy;
-        info->state = sev_guest->state;
-        info->handle = sev_guest->handle;
+        if (sev_guest) {
+            info->handle = sev_guest->handle;

target/i386/sev.c static struct RAMBlockNotifier sev_ram_notifier = { · line 351

Quoted 10 lines

target/i386/sev.c +9 -0

+        }
+        info->api_major = sev_common->api_major;
+        info->api_minor = sev_common->api_minor;
+        info->build_id = sev_common->build_id;
+        info->state = sev_common->state;
+        /* we only report the lower 32-bits of policy for SNP, ok for now... */
+        info->policy =
+            (uint32_t)object_property_get_uint(OBJECT(sev_common),
+                                               "policy", NULL);
     }

I think we can change this 'policy' field to 'int64'.