kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU
@ 2021-05-06  1:40 Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 01/10] Extend the MemTxAttrs to include a 'debug' flag. The flag can be used as general indicator that operation was triggered by the debugger Yuan Yao
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Yuan Yao <yuan.yao@intel.com>

This RFC series introduces the basic framework and a common
implementation on x86 to handle encrypted guest memory
reading/writing, to support QEMU's built-in guest debugging
features, like the monitor command xp and gdbstub.

The encrypted guest which its memory and/or register context
is encrypted by vendor specific technology(AMD SEV/INTEL TDX),
is able to resist the attack from malicious VMM or other
privileged components in host side, however, this ability also
breaks down the QEMU's built-in guest debugging features,
because it prohibits the direct guest memory accessing
(memcpy() with HVA) from QEMU which is the base of these
debugging features.

The framework part based on the previous patche set from
AMD[1] and some discussion result in community[2]. The main
idea is, introduce some new debug interfaces to handle the
encrypted guest physical memory accessing, also introduce
new interfaces in MemoryRegion to handle the actual accessing
there with KVM, don't bother the exist memory access logic or
callbacks as far as possible. 

[1] https://lore.kernel.org/qemu-devel/
    cover.1605316268.git.ashish.kalra@amd.com/
[2] https://lore.kernel.org/qemu-devel/
    20200922201124.GA6606@ashkalra_ubuntu_server/

 - The difference part in this patch series:
   - We introduce another new vm level ioctl focus on the encrypted
     guest memory accessing:

     KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY

     struct kvm_rw_memory rw;
     rw.addr = gpa_OR_hva;
     rw.buf = (__u64)src;
     rw.len = len;
     kvm_vm_ioctl(kvm_state,
                  KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY,
                  &rw);

     This new ioctl has more neutral and general name for its
     purpose, the debugging support of AMD SEV and INTEL TDX
     can be covered by a unify QEMU implementation on x86 with this
     ioctl. Although only INTEL TD guest is supported in this series,
     AMD SEV could be also supported with implementation of this
     ioctl in KVM, plus small modifications in QEMU to enable the
     unify part.

   - The MemoryRegion interface introduced by AMD before now has
     addtional GPA parameter(only HVA before).
     This is for INTEL TDX which uses GPA to do guest memory
     accessing. This change won't impact AMD SEV which is using
     HVA to access the guest memory.

 - New APIs in QEMU:
   - Physical memory accessing:
     - cpu_physical_memory_rw_debug().
     - cpu_physical_memory_read_debug().
     - cpu_physical_memory_write_debug().
     - x86_ldl_phys_debug().
     - x86_ldq_phys_debug().
   - Access from address_space:
     - address_space_read_debug().
     - address_space_write_rom_debug().
   - Virtual memory accessing and page table walking:
     - cpu_memory_rw_debug().
     - x86_cpu_get_phys_page_attrs_encrypted_debug().

 - New intrfaces in QEMU:
   - MemoryDebugOps *physical_memory_debug_op
     - For normal guest:
       Just call the old exist memory RW functions.
     - For encrypted guest:
       Forward the request to MemoryRegion->ram_debug_ops

   - MemoryRegionRAMReadWriteOps MemoryRegion::*ram_debug_ops
     - For normal guest:
       NULL and nobody use it.
     - For encrypted guest:
       Forward the request to common/vendor specific implementation.

 - The relationship diagram of the APIs and interfaces:

                 +---------------------------------------------+
                 |x86_cpu_get_phys_page_attrs_encrypted_debug()|
                 +----------------------------------+----------+
                                                    |
          +---------------------------------+       |
          |cpu_physical_memory_rw_debug()   |       |
          |cpu_physical_memory_read_debug() |       |
          |cpu_physical_memory_write_debug()|       |
          +----------------------+----------+       |
                                 |                  |
   +---------------------+       |        +---------v----------+
   |cpu_memory_rw_debug()|       |        |x86_ldl_phys_debug()|
   +-------------------+-+       |        |x86_ldq_phys_debug()|
                       |         |        +-------+------------+
                       |         |                |
                       |         |                |
  +--------------------v---------v----------------v------------+
  |         MemoryDebugOps *physical_memory_debug_op           |
  +----------------------+--------------------------+----------+
                         |                          |
                         |Encrypted guest           |Normal guest
                         |                          |
    +--------------------v-----------------------+  |
    |address_space_encrypted_memory_read_debug() |  |
    |address_space_encrypted_rom_write_debug()   |  |
    +--------------------+-----------------------+  |
                         |                          | 
                         |          +---------------v----------+
                         |          |address_space_read()      |
                         |          |address_space_write_rom() |
                         |          +--------------------------+
                         |
        +----------------v----------------+
        | address_space_read_debug()      |
        | address_space_write_rom_debug() |
        +----------------+----------------+
                         |
                         |
                         |
        +----------------v----------------+
        |  MemoryRegionRAMReadWriteOps    |
        |  MemoryRegion::*ram_debug_ops   |
        +--------+--------------+---------+
                 |              |
                 |              |Normal guest
                 |              |
  Encrypted guest|          +---v-------------------+
                 |          | NULL(nobody using it) |
                 |          +-----------------------+
                 |
       +---------v----------------------------+
       |  kvm_encrypted_guest_read_memory()   |
       |  kvm_encrypted_guest_write_memory()  |
       +--------------------------------------+

Ashish Kalra (2):
  Introduce new MemoryDebugOps which hook into guest virtual and
    physical memory debug interfaces such as cpu_memory_rw_debug, to
    allow vendor specific assist/hooks for debugging and delegating
    accessing the guest memory. This is required for example in case of
    AMD SEV platform where the guest memory is encrypted and a SEV
    specific debug assist/hook will be required to access the guest
    memory.
  Add new address_space_read and address_space_write debug helper
    interfaces which can be invoked by vendor specific guest memory
    debug assist/hooks to do guest RAM memory accesses using the added
    MemoryRegion callbacks.

Brijesh Singh (2):
  Extend the MemTxAttrs to include a 'debug' flag. The flag can be used
    as general indicator that operation was triggered by the debugger.
  Currently, guest memory access for debugging purposes is performed
    using memcpy(). Extend the 'struct MemoryRegion' to include new
    callbacks that can be used to override the use of memcpy() with
    something else.

Yuan Yao (6):
  Introduce new interface KVMState::set_mr_debug_ops and its wrapper
  Implements the common MemoryRegion::ram_debug_ops for encrypted guests
  Set the RAM's MemoryRegion::debug_ops for INTEL TD guests
  Introduce debug version of physical memory read/write API
  Change the monitor and other commands and gdbstub to use the debug API
  Introduce new CPUClass::get_phys_page_attrs_debug implementation for
    encrypted guests

 accel/kvm/kvm-all.c       |  17 +++++
 accel/stubs/kvm-stub.c    |  11 +++
 dump/dump.c               |   2 +-
 gdbstub.c                 |   4 +-
 hw/i386/pc.c              |   4 +
 include/exec/cpu-common.h |  14 ++++
 include/exec/memattrs.h   |   4 +
 include/exec/memory.h     |  54 +++++++++++++
 include/sysemu/kvm.h      |   5 ++
 include/sysemu/tdx.h      |   3 +
 monitor/misc.c            |  12 ++-
 softmmu/cpus.c            |   2 +-
 softmmu/physmem.c         | 154 +++++++++++++++++++++++++++++++++++++-
 target/i386/cpu.h         |   4 +
 target/i386/helper.c      |  64 +++++++++++++---
 target/i386/kvm/kvm.c     |  68 +++++++++++++++++
 target/i386/kvm/tdx.c     |  21 ++++++
 target/i386/monitor.c     |  52 ++++++-------
 18 files changed, 447 insertions(+), 48 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 01/10] Extend the MemTxAttrs to include a 'debug' flag. The flag can be used as general indicator that operation was triggered by the debugger.
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 02/10] Currently, guest memory access for debugging purposes is performed using memcpy(). Extend the 'struct MemoryRegion' to include new callbacks that can be used to override the use of memcpy() with something else Yuan Yao
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Brijesh Singh <brijesh.singh@amd.com>

A subsequent patch will set the debug=1 when issuing a memory access
from the gdbstub or HMP commands. This is a prerequisite to support
debugging an encrypted guest. When a request with debug=1 is seen, the
encryption APIs will be used to access the guest memory.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>

diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 95f2d20d55..c8b56389d6 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -49,6 +49,8 @@ typedef struct MemTxAttrs {
     unsigned int target_tlb_bit0 : 1;
     unsigned int target_tlb_bit1 : 1;
     unsigned int target_tlb_bit2 : 1;
+    /* Memory access request from the debugger */
+    unsigned int debug:1;
 } MemTxAttrs;
 
 /* Bus masters which don't specify any attributes will get this,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 02/10] Currently, guest memory access for debugging purposes is performed using memcpy(). Extend the 'struct MemoryRegion' to include new callbacks that can be used to override the use of memcpy() with something else.
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 01/10] Extend the MemTxAttrs to include a 'debug' flag. The flag can be used as general indicator that operation was triggered by the debugger Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 03/10] Introduce new interface KVMState::set_mr_debug_ops and its wrapper Yuan Yao
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Brijesh Singh <brijesh.singh@amd.com>

The new callbacks can be used to display the guest memory of an SEV guest
by registering callbacks to the SEV memory encryption/decryption APIs.

Typical usage:

mem_read(uint8_t *dest,
         const uint8_t *hva_src, hwaddr gpa_src,
         uint32_t len, MemTxAttrs attrs);
mem_write(uint8_t *hva_dest, hwaddr gpa_des,
          const uint8_t *src, uint32_t len, MemTxAttrs attrs);

MemoryRegionRAMReadWriteOps ops;
ops.read = mem_read;
ops.write = mem_write;

memory_region_init_ram(mem, NULL, "memory", size, NULL);
memory_region_set_ram_debug_ops(mem, ops);

Yuan Yao:
 - Add the gpa_src/gpa_des for read/write interface

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5728a681b2..7e6fdcb8e4 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -444,6 +444,19 @@ struct IOMMUMemoryRegionClass {
 typedef struct CoalescedMemoryRange CoalescedMemoryRange;
 typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
 
+/* Memory Region RAM debug callback */
+typedef struct MemoryRegionRAMReadWriteOps MemoryRegionRAMReadWriteOps;
+
+struct MemoryRegionRAMReadWriteOps {
+    /* Write data into guest memory */
+    int (*write) (uint8_t *hva_dest, hwaddr gpa_des,
+                  const uint8_t *src, uint32_t len, MemTxAttrs attrs);
+    /* Read data from guest memory */
+    int (*read) (uint8_t *dest,
+                 const uint8_t *hva_src, hwaddr gpa_src,
+                 uint32_t len, MemTxAttrs attrs);
+};
+
 /** MemoryRegion:
  *
  * A struct representing a memory region.
@@ -487,6 +500,7 @@ struct MemoryRegion {
     const char *name;
     unsigned ioeventfd_nb;
     MemoryRegionIoeventfd *ioeventfds;
+    const MemoryRegionRAMReadWriteOps *ram_debug_ops;
 };
 
 struct IOMMUMemoryRegion {
@@ -1130,6 +1144,20 @@ void memory_region_init_rom_nomigrate(MemoryRegion *mr,
                                       uint64_t size,
                                       Error **errp);
 
+/**
+ * memory_region_set_ram_debug_ops: Set access ops for a give memory region.
+ *
+ * @mr: the #MemoryRegion to be initialized
+ * @ops: a function that will be used when accessing @target region during
+ *       debug
+ */
+static inline void
+memory_region_set_ram_debug_ops(MemoryRegion *mr,
+                                const MemoryRegionRAMReadWriteOps *ops)
+{
+    mr->ram_debug_ops = ops;
+}
+
 /**
  * memory_region_init_rom_device_nomigrate:  Initialize a ROM memory region.
  *                                 Writes are handled via callbacks.
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 03/10] Introduce new interface KVMState::set_mr_debug_ops and its wrapper
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 01/10] Extend the MemTxAttrs to include a 'debug' flag. The flag can be used as general indicator that operation was triggered by the debugger Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 02/10] Currently, guest memory access for debugging purposes is performed using memcpy(). Extend the 'struct MemoryRegion' to include new callbacks that can be used to override the use of memcpy() with something else Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 04/10] Implements the common MemoryRegion::ram_debug_ops for encrypted guests Yuan Yao
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Yuan Yao <yuan.yao@intel.com>

This interface is designed to setup the MemoryRegion::debug_ops.

Also introduced 2 wrapper functions for installing/calling the
KVMState::set_mr_debug_ops from different targets easily.

Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index b6d5c9fd7d..1482561bd7 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -129,6 +129,8 @@ struct KVMState
         KVMMemoryListener *ml;
         AddressSpace *as;
     } *as;
+
+    set_memory_region_debug_ops set_mr_debug_ops;
 };
 
 KVMState *kvm_state;
@@ -3157,6 +3159,21 @@ static void kvm_set_kernel_irqchip(Object *obj, Visitor *v,
     }
 }
 
+void kvm_setup_memory_region_debug_ops(struct KVMState *s,
+                                       set_memory_region_debug_ops new_ops)
+{
+    if (s)
+        s->set_mr_debug_ops = new_ops;
+}
+
+void kvm_set_memory_region_debug_ops(void *handle, MemoryRegion *mr)
+{
+    if (!kvm_state || !kvm_state->set_mr_debug_ops)
+        return;
+
+    kvm_state->set_mr_debug_ops(handle, mr);
+}
+
 bool kvm_kernel_irqchip_allowed(void)
 {
     return kvm_state->kernel_irqchip_allowed;
diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
index 0f17acfac0..f1c57ad8d7 100644
--- a/accel/stubs/kvm-stub.c
+++ b/accel/stubs/kvm-stub.c
@@ -148,4 +148,15 @@ bool kvm_arm_supports_user_irq(void)
 {
     return false;
 }
+
+void kvm_setup_memory_region_debug_ops(struct KVMState *s,
+                                       set_memory_region_debug_ops new_ops)
+{
+
+}
+
+void kvm_set_memory_region_debug_ops(void *handle, MemoryRegion *mr)
+{
+
+}
 #endif
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d..64685cad57 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -547,4 +547,9 @@ bool kvm_cpu_check_are_resettable(void);
 
 bool kvm_arch_cpu_check_are_resettable(void);
 
+typedef void (*set_memory_region_debug_ops)(void *handle, MemoryRegion *mr);
+void kvm_setup_memory_region_debug_ops(struct KVMState *s, set_memory_region_debug_ops new_ops);
+
+void kvm_set_memory_region_debug_ops(void *handle, MemoryRegion *mr);
+
 #endif
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 04/10] Implements the common MemoryRegion::ram_debug_ops for encrypted guests
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (2 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 03/10] Introduce new interface KVMState::set_mr_debug_ops and its wrapper Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 05/10] Set the RAM's MemoryRegion::debug_ops for INTEL TD guests Yuan Yao
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Yuan Yao <yuan.yao@intel.com>

The new functions are added into target/i386/kvm/kvm.c as common functions
to support encrypted guest for KVM on x86.

Now we enable these only for INTEL TD guests.

Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 05bf4f8b8b..5050b2a82f 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -134,6 +134,9 @@ static struct kvm_msr_list *kvm_feature_msrs;
 
 static int vm_type;
 
+void kvm_encrypted_guest_set_memory_region_debug_ops(void *handle,
+                                                     MemoryRegion *mr);
+
 int kvm_set_vm_type(MachineState *ms, int kvm_type)
 {
     if (kvm_type == KVM_X86_LEGACY_VM ||
@@ -2228,6 +2231,10 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         return ret;
     }
 
+    if (kvm_tdx_enabled())
+        kvm_setup_memory_region_debug_ops(s,
+                                          kvm_encrypted_guest_set_memory_region_debug_ops);
+
     if (!kvm_check_extension(s, KVM_CAP_IRQ_ROUTING)) {
         error_report("kvm: KVM_CAP_IRQ_ROUTING not supported by KVM");
         return -ENOTSUP;
@@ -4917,3 +4924,62 @@ bool kvm_arch_cpu_check_are_resettable(void)
 {
     return !sev_es_enabled();
 }
+
+static int kvm_encrypted_guest_read_memory(uint8_t *dest,
+                                           const uint8_t *hva_src, hwaddr gpa_src,
+                                           uint32_t len, MemTxAttrs attrs)
+{
+    struct kvm_rw_memory rw;
+
+    /*
+      TODO:
+      Can we check SEV/TDX state to decide use
+      gpa_dest or hva_dest here ?
+
+      Also how shall we handle the kvm_vm_ioctl failure case ?
+      Some user like cpu_physical_memory_{read,write}() doesn't handle such
+      failure, because for non-encrypted guest these functions may do memory
+      reading/wrting with memcpy() dirctly before.
+      May memset() the buffer to a bad pattern (all 0x0 or 0xff)
+      for indicating this ?
+    */
+    rw.addr = gpa_src;
+    rw.buf = dest;
+    rw.len = len;
+
+    return kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_READ_MEMORY, &rw);
+}
+
+static int kvm_encrypted_guest_write_memory(uint8_t *hva_dest, hwaddr gpa_dest,
+                                            const uint8_t *src,
+                                            uint32_t len, MemTxAttrs attrs)
+{
+    struct kvm_rw_memory rw;
+
+    /*
+      TODO:
+      Can we check SEV/TDX state to decide use
+      gpa_dest or hva_dest here ?
+
+      Also how shall we handle the kvm_vm_ioctl failure case ?
+      Some user like cpu_physical_memory_{read,write}() doesn't handle such
+      failure, because for non-encrypted guest these functions may do memory
+      reading/wrting with memcpy() dirctly before.
+     */
+    rw.addr = gpa_dest;
+    rw.buf = (void*)src;
+    rw.len = len;
+
+    return kvm_vm_ioctl(kvm_state, KVM_MEMORY_ENCRYPT_WRITE_MEMORY, &rw);
+}
+
+static MemoryRegionRAMReadWriteOps kvm_encrypted_guest_mr_debug_ops = {
+    .read = kvm_encrypted_guest_read_memory,
+    .write = kvm_encrypted_guest_write_memory,
+};
+
+void kvm_encrypted_guest_set_memory_region_debug_ops(void *handle,
+                                                     MemoryRegion *mr)
+{
+    memory_region_set_ram_debug_ops(mr, &kvm_encrypted_guest_mr_debug_ops);
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 05/10] Set the RAM's MemoryRegion::debug_ops for INTEL TD guests
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (3 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 04/10] Implements the common MemoryRegion::ram_debug_ops for encrypted guests Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 06/10] Introduce new MemoryDebugOps which hook into guest virtual and physical memory debug interfaces such as cpu_memory_rw_debug, to allow vendor specific assist/hooks for debugging and delegating accessing the guest memory. This is required for example in case of AMD SEV platform where the guest memory is encrypted and a SEV specific debug assist/hook will be required to access the guest memory Yuan Yao
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Yuan Yao <yuan.yao@intel.com>

Now only set the RAM's debug_ops for INTEL TD guests, SEV can also
rely on the common part introduced in previous patch or introduce
new debug_ops implementation if it's necessary.

Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d5a4345f44..772b19c524 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -60,6 +60,7 @@
 #include "sysemu/xen.h"
 #include "sysemu/reset.h"
 #include "sysemu/runstate.h"
+#include "sysemu/tdx.h"
 #include "kvm/kvm_i386.h"
 #include "hw/xen/xen.h"
 #include "hw/xen/start_info.h"
@@ -992,6 +993,9 @@ void pc_memory_init(PCMachineState *pcms,
 
     /* Init ACPI memory hotplug IO base address */
     pcms->memhp_io_base = ACPI_MEMORY_HOTPLUG_BASE;
+
+    if (tdx_debug_enabled(machine->cgs))
+        kvm_set_memory_region_debug_ops(NULL, *ram_memory);
 }
 
 /*
diff --git a/include/sysemu/tdx.h b/include/sysemu/tdx.h
index 429bb0ff8e..bd0af77c03 100644
--- a/include/sysemu/tdx.h
+++ b/include/sysemu/tdx.h
@@ -16,4 +16,7 @@ void tdx_post_init_vcpu(CPUState *cpu);
 struct TDXCapability;
 struct TDXCapability *tdx_get_capabilities(void);
 
+struct ConfidentialGuestSupport;
+bool tdx_debug_enabled(ConfidentialGuestSupport *cgs);
+
 #endif
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index c4e5686260..d13d4c8487 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -384,3 +384,18 @@ static void tdx_guest_finalize(Object *obj)
 static void tdx_guest_class_init(ObjectClass *oc, void *data)
 {
 }
+
+bool tdx_debug_enabled(ConfidentialGuestSupport *cgs)
+{
+    TdxGuest *tdx;
+
+    if (!cgs)
+        return false;
+
+    tdx = (TdxGuest *)object_dynamic_cast(OBJECT(cgs),
+                                          TYPE_TDX_GUEST);
+    if (!tdx)
+        return false;
+
+    return tdx->debug;
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 06/10] Introduce new MemoryDebugOps which hook into guest virtual and physical memory debug interfaces such as cpu_memory_rw_debug, to allow vendor specific assist/hooks for debugging and delegating accessing the guest memory. This is required for example in case of AMD SEV platform where the guest memory is encrypted and a SEV specific debug assist/hook will be required to access the guest memory.
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (4 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 05/10] Set the RAM's MemoryRegion::debug_ops for INTEL TD guests Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 07/10] Add new address_space_read and address_space_write debug helper interfaces which can be invoked by vendor specific guest memory debug assist/hooks to do guest RAM memory accesses using the added MemoryRegion callbacks Yuan Yao
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Ashish Kalra <ashish.kalra@amd.com>

The MemoryDebugOps are used by cpu_memory_rw_debug() and default to
address_space_read and address_space_write_rom.

Yuan Yao: Exports the physical_memory_debug_ops variable for functions
in target/i386/helper.c

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7e6fdcb8e4..0250b50beb 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2498,6 +2498,20 @@ MemTxResult address_space_write_cached_slow(MemoryRegionCache *cache,
                                             hwaddr addr, const void *buf,
                                             hwaddr len);
 
+typedef struct MemoryDebugOps {
+    MemTxResult (*read)(AddressSpace *as, hwaddr phys_addr,
+                        MemTxAttrs attrs, void *buf,
+                        hwaddr len);
+    MemTxResult (*write)(AddressSpace *as, hwaddr phys_addr,
+                         MemTxAttrs attrs, const void *buf,
+                         hwaddr len);
+} MemoryDebugOps;
+
+// Export for functions in target/i386/helper.c
+extern const MemoryDebugOps *physical_memory_debug_ops;
+
+void address_space_set_debug_ops(const MemoryDebugOps *ops);
+
 static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
 {
     if (is_write) {
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 85034d9c11..c8029f69ad 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -171,6 +171,18 @@ struct DirtyBitmapSnapshot {
     unsigned long dirty[];
 };
 
+static const MemoryDebugOps default_debug_ops = {
+    .read = address_space_read,
+    .write = address_space_write_rom
+};
+
+const MemoryDebugOps *physical_memory_debug_ops = &default_debug_ops;
+
+void address_space_set_debug_ops(const MemoryDebugOps *ops)
+{
+    physical_memory_debug_ops = ops;
+}
+
 static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes)
 {
     static unsigned alloc_hint = 16;
@@ -3396,6 +3408,10 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
         page = addr & TARGET_PAGE_MASK;
         phys_addr = cpu_get_phys_page_attrs_debug(cpu, page, &attrs);
         asidx = cpu_asidx_from_attrs(cpu, attrs);
+
+        /* set debug attrs to indicate memory access is from the debugger */
+        attrs.debug = 1;
+
         /* if no physical page mapped, return an error */
         if (phys_addr == -1)
             return -1;
@@ -3404,11 +3420,13 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
             l = len;
         phys_addr += (addr & ~TARGET_PAGE_MASK);
         if (is_write) {
-            res = address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr,
-                                          attrs, buf, l);
+            res = physical_memory_debug_ops->write(cpu->cpu_ases[asidx].as,
+                                                   phys_addr,
+                                                   attrs, buf, l);
         } else {
-            res = address_space_read(cpu->cpu_ases[asidx].as, phys_addr,
-                                     attrs, buf, l);
+            res = physical_memory_debug_ops->read(cpu->cpu_ases[asidx].as,
+                                                  phys_addr,
+                                                  attrs, buf, l);
         }
         if (res != MEMTX_OK) {
             return -1;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 07/10] Add new address_space_read and address_space_write debug helper interfaces which can be invoked by vendor specific guest memory debug assist/hooks to do guest RAM memory accesses using the added MemoryRegion callbacks.
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (5 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 06/10] Introduce new MemoryDebugOps which hook into guest virtual and physical memory debug interfaces such as cpu_memory_rw_debug, to allow vendor specific assist/hooks for debugging and delegating accessing the guest memory. This is required for example in case of AMD SEV platform where the guest memory is encrypted and a SEV specific debug assist/hook will be required to access the guest memory Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 08/10] Introduce debug version of physical memory read/write API Yuan Yao
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Ashish Kalra <ashish.kalra@amd.com>

Yuan Yao:
    - Fixed fuzz_dma_read_cb() parameter issue for QEMU 5.2.91.
    - Move the caller of encrypted_memory_debug_ops into phymem.c
      as common callbacks for encrypted guests.
    - Adapted address_space_read_debug/address_space_wirte_rom_debug
      with new definition of MemoryRegion::ram_debug_ops;
    - Install the encrypted_memory_debug_ops/phymem.c for INTEL
      TD guest.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0250b50beb..c0d6c1bd8f 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2246,6 +2246,12 @@ MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
                                     MemTxAttrs attrs,
                                     const void *buf, hwaddr len);
 
+MemTxResult address_space_write_rom_debug(AddressSpace *as,
+                                          hwaddr addr,
+                                          MemTxAttrs attrs,
+                                          const void *ptr,
+                                          hwaddr len);
+
 /* address_space_ld*: load from an address space
  * address_space_st*: store to an address space
  *
@@ -2512,6 +2518,8 @@ extern const MemoryDebugOps *physical_memory_debug_ops;
 
 void address_space_set_debug_ops(const MemoryDebugOps *ops);
 
+void set_encrypted_memory_debug_ops(void);
+
 static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
 {
     if (is_write) {
@@ -2567,6 +2575,10 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
     return result;
 }
 
+MemTxResult address_space_read_debug(AddressSpace *as, hwaddr addr,
+                                     MemTxAttrs attrs, void *buf,
+                                     hwaddr len);
+
 /**
  * address_space_read_cached: read from a cached RAM region
  *
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index c8029f69ad..0fde02d325 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3245,6 +3245,94 @@ void cpu_physical_memory_unmap(void *buffer, hwaddr len,
 #define RCU_READ_UNLOCK(...)     rcu_read_unlock()
 #include "memory_ldst.c.inc"
 
+inline MemTxResult address_space_read_debug(AddressSpace *as, hwaddr addr,
+                                            MemTxAttrs attrs, void *ptr,
+                                            hwaddr len)
+{
+    uint64_t val;
+    MemoryRegion *mr;
+    hwaddr l = len;
+    hwaddr addr1;
+    MemTxResult result = MEMTX_OK;
+    bool release_lock = false;
+    uint8_t *buf = ptr;
+    uint8_t *ram_ptr;
+
+    for (;;) {
+        RCU_READ_LOCK_GUARD();
+        mr = address_space_translate(as, addr, &addr1, &l, false, attrs);
+        if (!memory_access_is_direct(mr, false)) {
+            /* I/O case */
+            release_lock |= prepare_mmio_access(mr);
+            l = memory_access_size(mr, l, addr1);
+            result |= memory_region_dispatch_read(mr, addr1, &val,
+                                                  size_memop(l), attrs);
+            stn_he_p(buf, l, val);
+        } else {
+            /* RAM case */
+            fuzz_dma_read_cb(addr, l, mr);
+            ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
+            if (attrs.debug && mr->ram_debug_ops) {
+                mr->ram_debug_ops->read(buf, ram_ptr, addr1, l, attrs);
+            } else {
+                memcpy(buf, ram_ptr, l);
+            }
+            result = MEMTX_OK;
+        }
+        if (release_lock) {
+            qemu_mutex_unlock_iothread();
+            release_lock = false;
+        }
+
+        len -= l;
+        buf += l;
+        addr += l;
+
+        if (!len) {
+            break;
+        }
+        l = len;
+    }
+    return result;
+}
+
+MemTxResult address_space_write_rom_debug(AddressSpace *as,
+                                          hwaddr addr,
+                                          MemTxAttrs attrs,
+                                          const void *ptr,
+                                          hwaddr len)
+{
+    hwaddr l;
+    uint8_t *ram_ptr;
+    hwaddr addr1;
+    MemoryRegion *mr;
+    const uint8_t *buf = ptr;
+
+    RCU_READ_LOCK_GUARD();
+    while (len > 0) {
+        l = len;
+        mr = address_space_translate(as, addr, &addr1, &l, true, attrs);
+
+        if (!(memory_region_is_ram(mr) ||
+              memory_region_is_romd(mr))) {
+            l = memory_access_size(mr, l, addr1);
+        } else {
+            /* ROM/RAM case */
+            ram_ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
+            if (attrs.debug && mr->ram_debug_ops) {
+                mr->ram_debug_ops->write(ram_ptr, addr1, buf, l, attrs);
+            } else {
+                memcpy(ram_ptr, buf, l);
+            }
+            invalidate_and_set_dirty(mr, addr1, l);
+        }
+        len -= l;
+        buf += l;
+        addr += l;
+    }
+    return MEMTX_OK;
+}
+
 int64_t address_space_cache_init(MemoryRegionCache *cache,
                                  AddressSpace *as,
                                  hwaddr addr,
@@ -3438,6 +3526,33 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
     return 0;
 }
 
+static MemTxResult address_space_encrypted_memory_read_debug(AddressSpace *as,
+                                                             hwaddr addr, MemTxAttrs attrs,
+                                                             void *ptr, hwaddr len)
+{
+    attrs.debug = 1;
+    return address_space_read_debug(as, addr, attrs, ptr, len);
+}
+
+
+static MemTxResult address_space_encrypted_rom_write_debug(AddressSpace *as,
+                                                           hwaddr addr, MemTxAttrs attrs,
+                                                           const void *ptr, hwaddr len)
+{
+    attrs.debug = 1;
+    return address_space_write_rom_debug(as, addr, attrs, ptr, len);
+}
+
+static const MemoryDebugOps encrypted_memory_debug_ops = {
+    .read = address_space_encrypted_memory_read_debug,
+    .write = address_space_encrypted_rom_write_debug,
+};
+
+void set_encrypted_memory_debug_ops(void)
+{
+    address_space_set_debug_ops(&encrypted_memory_debug_ops);
+}
+
 /*
  * Allows code that needs to deal with migration bitmaps etc to still be built
  * target independent.
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 5050b2a82f..228d18a449 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2231,9 +2231,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         return ret;
     }
 
-    if (kvm_tdx_enabled())
+    if (kvm_tdx_enabled()) {
         kvm_setup_memory_region_debug_ops(s,
                                           kvm_encrypted_guest_set_memory_region_debug_ops);
+        set_encrypted_memory_debug_ops();
+    }
 
     if (!kvm_check_extension(s, KVM_CAP_IRQ_ROUTING)) {
         error_report("kvm: KVM_CAP_IRQ_ROUTING not supported by KVM");
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 08/10] Introduce debug version of physical memory read/write API
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (6 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 07/10] Add new address_space_read and address_space_write debug helper interfaces which can be invoked by vendor specific guest memory debug assist/hooks to do guest RAM memory accesses using the added MemoryRegion callbacks Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 09/10] Change the monitor and other commands and gdbstub to use the debug API Yuan Yao
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Yuan Yao <yuan.yao@intel.com>

Add below APIs for reading/writing the physical memory, subsequent
patch will use them in monitor commands and gdbstub to support
encrypted guest debugging.

uint32_t x86_ldl_phys_debug(CPUState *cs, hwaddr addr);
uint64_t x86_ldq_phys_debug(CPUState *cs, hwaddr addr);
void cpu_physical_memory_rw_debug(hwaddr addr, void *buf,
                                  hwaddr len, bool is_write);
void cpu_physical_memory_read_debug(hwaddr addr,
                                    void *buf,
                                    hwaddr len);
void cpu_physical_memory_write_debug(hwaddr addr,
                                     const void *buf,
                                     hwaddr len);

Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 5a0a2d93e0..f77a9ecb60 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -69,6 +69,8 @@ size_t qemu_ram_pagesize_largest(void);
 
 void cpu_physical_memory_rw(hwaddr addr, void *buf,
                             hwaddr len, bool is_write);
+void cpu_physical_memory_rw_debug(hwaddr addr, void *buf,
+                                  hwaddr len, bool is_write);
 static inline void cpu_physical_memory_read(hwaddr addr,
                                             void *buf, hwaddr len)
 {
@@ -79,6 +81,18 @@ static inline void cpu_physical_memory_write(hwaddr addr,
 {
     cpu_physical_memory_rw(addr, (void *)buf, len, true);
 }
+
+static inline void cpu_physical_memory_read_debug(hwaddr addr,
+                                                  void *buf, hwaddr len)
+{
+    cpu_physical_memory_rw_debug(addr, buf, len, false);
+}
+static inline void cpu_physical_memory_write_debug(hwaddr addr,
+                                                   const void *buf, hwaddr len)
+{
+    cpu_physical_memory_rw_debug(addr, (void *)buf, len, true);
+}
+
 void *cpu_physical_memory_map(hwaddr addr,
                               hwaddr *plen,
                               bool is_write);
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index c8b56389d6..6d223ea196 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -60,6 +60,9 @@ typedef struct MemTxAttrs {
  */
 #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
 
+// Same as MEMTXATTRS_UNSPECIFIED but enable debug
+#define MEMTXATTRS_UNSPECIFIED_DEBUG ((MemTxAttrs) { .unspecified = 1, .debug = 1 })
+
 /* New-style MMIO accessors can indicate that the transaction failed.
  * A zero (MEMTX_OK) response means success; anything else is a failure
  * of some kind. The memory subsystem will bitwise-OR together results
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 0fde02d325..ff6e215a3a 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2910,6 +2910,19 @@ void cpu_physical_memory_rw(hwaddr addr, void *buf,
                      buf, len, is_write);
 }
 
+void cpu_physical_memory_rw_debug(hwaddr addr, void *buf,
+                            hwaddr len, bool is_write)
+{
+    if (is_write)
+        physical_memory_debug_ops->write(&address_space_memory,
+                                         addr, MEMTXATTRS_UNSPECIFIED_DEBUG,
+                                         buf, len);
+    else
+        physical_memory_debug_ops->read(&address_space_memory,
+                                        addr, MEMTXATTRS_UNSPECIFIED_DEBUG,
+                                        buf, len);
+}
+
 enum write_rom_type {
     WRITE_DATA,
     FLUSH_CACHE,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index e5dbe84d3a..7a8a1386fb 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1960,6 +1960,8 @@ void x86_stl_phys_notdirty(CPUState *cs, hwaddr addr, uint32_t val);
 void x86_stw_phys(CPUState *cs, hwaddr addr, uint32_t val);
 void x86_stl_phys(CPUState *cs, hwaddr addr, uint32_t val);
 void x86_stq_phys(CPUState *cs, hwaddr addr, uint64_t val);
+uint32_t x86_ldl_phys_debug(CPUState *cs, hwaddr addr);
+uint64_t x86_ldq_phys_debug(CPUState *cs, hwaddr addr);
 #endif
 
 /* will be suppressed */
diff --git a/target/i386/helper.c b/target/i386/helper.c
index 618ad1c409..21edcb9204 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -663,4 +663,30 @@ void x86_stq_phys(CPUState *cs, hwaddr addr, uint64_t val)
 
     address_space_stq(as, addr, val, attrs, NULL);
 }
+
+uint32_t x86_ldl_phys_debug(CPUState *cs, hwaddr addr)
+{
+    uint32_t ret;
+    MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED_DEBUG;
+    int as_id = cpu_asidx_from_attrs(cs, attrs);
+    struct AddressSpace *as = cpu_get_address_space(cs, as_id);
+
+    physical_memory_debug_ops->read(as, addr, attrs,
+                                    &ret, sizeof(ret));
+
+    return tswap32(ret);
+}
+
+uint64_t x86_ldq_phys_debug(CPUState *cs, hwaddr addr)
+{
+    uint64_t ret;
+    MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED_DEBUG;
+    int as_id = cpu_asidx_from_attrs(cs, attrs);
+    struct AddressSpace *as = cpu_get_address_space(cs, as_id);
+
+    physical_memory_debug_ops->read(as, addr, attrs,
+                                    &ret, sizeof(ret));
+
+    return tswap64(ret);
+}
 #endif
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 09/10] Change the monitor and other commands and gdbstub to use the debug API
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (7 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 08/10] Introduce debug version of physical memory read/write API Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-05-06  1:40 ` [RFC][PATCH v1 10/10] Introduce new CPUClass::get_phys_page_attrs_debug implementation for encrypted guests Yuan Yao
  2021-09-02 14:04 ` [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Ashish Kalra
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Yuan Yao <yuan.yao@intel.com>

Please comment if some changes are incorrect or I missed something here.

Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/dump/dump.c b/dump/dump.c
index 929138e91d..21eb018092 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -1746,7 +1746,7 @@ static void dump_init(DumpState *s, int fd, bool has_format,
             warn_report("guest note format is unsupported: %" PRIu16, format);
         } else {
             s->guest_note = g_malloc(size + 1); /* +1 for adding \0 */
-            cpu_physical_memory_read(addr, s->guest_note, size);
+            cpu_physical_memory_read_debug(addr, s->guest_note, size);
 
             get_note_sizes(s, s->guest_note, NULL, &name_size, &desc_size);
             s->guest_note_size = ELF_NOTE_SIZE(note_head_size, name_size,
diff --git a/gdbstub.c b/gdbstub.c
index 054665e93e..bdb3c688e5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -71,9 +71,9 @@ static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr,
 #ifndef CONFIG_USER_ONLY
     if (phy_memory_mode) {
         if (is_write) {
-            cpu_physical_memory_write(addr, buf, len);
+            cpu_physical_memory_write_debug(addr, buf, len);
         } else {
-            cpu_physical_memory_read(addr, buf, len);
+            cpu_physical_memory_read_debug(addr, buf, len);
         }
         return 0;
     }
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 6d223ea196..e62d135829 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -59,7 +59,6 @@ typedef struct MemTxAttrs {
  * from "didn't specify" if necessary).
  */
 #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
-
 // Same as MEMTXATTRS_UNSPECIFIED but enable debug
 #define MEMTXATTRS_UNSPECIFIED_DEBUG ((MemTxAttrs) { .unspecified = 1, .debug = 1 })
 
diff --git a/monitor/misc.c b/monitor/misc.c
index 55f3744053..60e4473af3 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -587,8 +587,9 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
             l = line_size;
         if (is_physical) {
             AddressSpace *as = cs ? cs->as : &address_space_memory;
-            MemTxResult r = address_space_read(as, addr,
-                                               MEMTXATTRS_UNSPECIFIED, buf, l);
+            MemTxResult r = address_space_read_debug(as, addr,
+                                                     MEMTXATTRS_UNSPECIFIED_DEBUG,
+                                                     buf, l);
             if (r != MEMTX_OK) {
                 monitor_printf(mon, " Cannot access memory\n");
                 break;
@@ -825,11 +826,14 @@ static void hmp_sum(Monitor *mon, const QDict *qdict)
     uint16_t sum;
     uint32_t start = qdict_get_int(qdict, "start");
     uint32_t size = qdict_get_int(qdict, "size");
+    uint8_t val;
 
     sum = 0;
     for(addr = start; addr < (start + size); addr++) {
-        uint8_t val = address_space_ldub(&address_space_memory, addr,
-                                         MEMTXATTRS_UNSPECIFIED, NULL);
+        address_space_read_debug(&address_space_memory,
+                                 addr,
+                                 MEMTXATTRS_UNSPECIFIED_DEBUG,
+                                 &val, sizeof(val));
         /* BSD sum algorithm ('sum' Unix command) */
         sum = (sum >> 1) | (sum << 15);
         sum += val;
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a7ee431187..baa2a2022a 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -785,7 +785,7 @@ void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
         l = sizeof(buf);
         if (l > size)
             l = size;
-        cpu_physical_memory_read(addr, buf, l);
+        cpu_physical_memory_read_debug(addr, buf, l);
         if (fwrite(buf, 1, l, f) != l) {
             error_setg(errp, QERR_IO_ERROR);
             goto exit;
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 2b636293c6..f9412b6030 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -81,7 +81,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
 
     pgd = env->cr[3] & ~0xfff;
     for(l1 = 0; l1 < 1024; l1++) {
-        cpu_physical_memory_read(pgd + l1 * 4, &pde, 4);
+        cpu_physical_memory_read_debug(pgd + l1 * 4, &pde, 4);
         pde = le32_to_cpu(pde);
         if (pde & PG_PRESENT_MASK) {
             if ((pde & PG_PSE_MASK) && (env->cr[4] & CR4_PSE_MASK)) {
@@ -89,7 +89,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
                 print_pte(mon, env, (l1 << 22), pde, ~((1 << 21) - 1));
             } else {
                 for(l2 = 0; l2 < 1024; l2++) {
-                    cpu_physical_memory_read((pde & ~0xfff) + l2 * 4, &pte, 4);
+                    cpu_physical_memory_read_debug((pde & ~0xfff) + l2 * 4, &pte, 4);
                     pte = le32_to_cpu(pte);
                     if (pte & PG_PRESENT_MASK) {
                         print_pte(mon, env, (l1 << 22) + (l2 << 12),
@@ -110,12 +110,12 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
 
     pdp_addr = env->cr[3] & ~0x1f;
     for (l1 = 0; l1 < 4; l1++) {
-        cpu_physical_memory_read(pdp_addr + l1 * 8, &pdpe, 8);
+        cpu_physical_memory_read_debug(pdp_addr + l1 * 8, &pdpe, 8);
         pdpe = le64_to_cpu(pdpe);
         if (pdpe & PG_PRESENT_MASK) {
             pd_addr = pdpe & 0x3fffffffff000ULL;
             for (l2 = 0; l2 < 512; l2++) {
-                cpu_physical_memory_read(pd_addr + l2 * 8, &pde, 8);
+                cpu_physical_memory_read_debug(pd_addr + l2 * 8, &pde, 8);
                 pde = le64_to_cpu(pde);
                 if (pde & PG_PRESENT_MASK) {
                     if (pde & PG_PSE_MASK) {
@@ -125,7 +125,7 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
                     } else {
                         pt_addr = pde & 0x3fffffffff000ULL;
                         for (l3 = 0; l3 < 512; l3++) {
-                            cpu_physical_memory_read(pt_addr + l3 * 8, &pte, 8);
+                            cpu_physical_memory_read_debug(pt_addr + l3 * 8, &pte, 8);
                             pte = le64_to_cpu(pte);
                             if (pte & PG_PRESENT_MASK) {
                                 print_pte(mon, env, (l1 << 30) + (l2 << 21)
@@ -150,7 +150,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
     uint64_t pdp_addr, pd_addr, pt_addr;
 
     for (l1 = 0; l1 < 512; l1++) {
-        cpu_physical_memory_read(pml4_addr + l1 * 8, &pml4e, 8);
+        cpu_physical_memory_read_debug(pml4_addr + l1 * 8, &pml4e, 8);
         pml4e = le64_to_cpu(pml4e);
         if (!(pml4e & PG_PRESENT_MASK)) {
             continue;
@@ -158,7 +158,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
 
         pdp_addr = pml4e & 0x3fffffffff000ULL;
         for (l2 = 0; l2 < 512; l2++) {
-            cpu_physical_memory_read(pdp_addr + l2 * 8, &pdpe, 8);
+            cpu_physical_memory_read_debug(pdp_addr + l2 * 8, &pdpe, 8);
             pdpe = le64_to_cpu(pdpe);
             if (!(pdpe & PG_PRESENT_MASK)) {
                 continue;
@@ -173,7 +173,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
 
             pd_addr = pdpe & 0x3fffffffff000ULL;
             for (l3 = 0; l3 < 512; l3++) {
-                cpu_physical_memory_read(pd_addr + l3 * 8, &pde, 8);
+                cpu_physical_memory_read_debug(pd_addr + l3 * 8, &pde, 8);
                 pde = le64_to_cpu(pde);
                 if (!(pde & PG_PRESENT_MASK)) {
                     continue;
@@ -188,7 +188,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
 
                 pt_addr = pde & 0x3fffffffff000ULL;
                 for (l4 = 0; l4 < 512; l4++) {
-                    cpu_physical_memory_read(pt_addr
+                    cpu_physical_memory_read_debug(pt_addr
                             + l4 * 8,
                             &pte, 8);
                     pte = le64_to_cpu(pte);
@@ -211,7 +211,7 @@ static void tlb_info_la57(Monitor *mon, CPUArchState *env)
 
     pml5_addr = env->cr[3] & 0x3fffffffff000ULL;
     for (l0 = 0; l0 < 512; l0++) {
-        cpu_physical_memory_read(pml5_addr + l0 * 8, &pml5e, 8);
+        cpu_physical_memory_read_debug(pml5_addr + l0 * 8, &pml5e, 8);
         pml5e = le64_to_cpu(pml5e);
         if (pml5e & PG_PRESENT_MASK) {
             tlb_info_la48(mon, env, l0, pml5e & 0x3fffffffff000ULL);
@@ -288,7 +288,7 @@ static void mem_info_32(Monitor *mon, CPUArchState *env)
     last_prot = 0;
     start = -1;
     for(l1 = 0; l1 < 1024; l1++) {
-        cpu_physical_memory_read(pgd + l1 * 4, &pde, 4);
+        cpu_physical_memory_read_debug(pgd + l1 * 4, &pde, 4);
         pde = le32_to_cpu(pde);
         end = l1 << 22;
         if (pde & PG_PRESENT_MASK) {
@@ -297,7 +297,7 @@ static void mem_info_32(Monitor *mon, CPUArchState *env)
                 mem_print(mon, env, &start, &last_prot, end, prot);
             } else {
                 for(l2 = 0; l2 < 1024; l2++) {
-                    cpu_physical_memory_read((pde & ~0xfff) + l2 * 4, &pte, 4);
+                    cpu_physical_memory_read_debug((pde & ~0xfff) + l2 * 4, &pte, 4);
                     pte = le32_to_cpu(pte);
                     end = (l1 << 22) + (l2 << 12);
                     if (pte & PG_PRESENT_MASK) {
@@ -330,13 +330,13 @@ static void mem_info_pae32(Monitor *mon, CPUArchState *env)
     last_prot = 0;
     start = -1;
     for (l1 = 0; l1 < 4; l1++) {
-        cpu_physical_memory_read(pdp_addr + l1 * 8, &pdpe, 8);
+        cpu_physical_memory_read_debug(pdp_addr + l1 * 8, &pdpe, 8);
         pdpe = le64_to_cpu(pdpe);
         end = l1 << 30;
         if (pdpe & PG_PRESENT_MASK) {
             pd_addr = pdpe & 0x3fffffffff000ULL;
             for (l2 = 0; l2 < 512; l2++) {
-                cpu_physical_memory_read(pd_addr + l2 * 8, &pde, 8);
+                cpu_physical_memory_read_debug(pd_addr + l2 * 8, &pde, 8);
                 pde = le64_to_cpu(pde);
                 end = (l1 << 30) + (l2 << 21);
                 if (pde & PG_PRESENT_MASK) {
@@ -347,7 +347,7 @@ static void mem_info_pae32(Monitor *mon, CPUArchState *env)
                     } else {
                         pt_addr = pde & 0x3fffffffff000ULL;
                         for (l3 = 0; l3 < 512; l3++) {
-                            cpu_physical_memory_read(pt_addr + l3 * 8, &pte, 8);
+                            cpu_physical_memory_read_debug(pt_addr + l3 * 8, &pte, 8);
                             pte = le64_to_cpu(pte);
                             end = (l1 << 30) + (l2 << 21) + (l3 << 12);
                             if (pte & PG_PRESENT_MASK) {
@@ -386,13 +386,13 @@ static void mem_info_la48(Monitor *mon, CPUArchState *env)
     last_prot = 0;
     start = -1;
     for (l1 = 0; l1 < 512; l1++) {
-        cpu_physical_memory_read(pml4_addr + l1 * 8, &pml4e, 8);
+        cpu_physical_memory_read_debug(pml4_addr + l1 * 8, &pml4e, 8);
         pml4e = le64_to_cpu(pml4e);
         end = l1 << 39;
         if (pml4e & PG_PRESENT_MASK) {
             pdp_addr = pml4e & 0x3fffffffff000ULL;
             for (l2 = 0; l2 < 512; l2++) {
-                cpu_physical_memory_read(pdp_addr + l2 * 8, &pdpe, 8);
+                cpu_physical_memory_read_debug(pdp_addr + l2 * 8, &pdpe, 8);
                 pdpe = le64_to_cpu(pdpe);
                 end = (l1 << 39) + (l2 << 30);
                 if (pdpe & PG_PRESENT_MASK) {
@@ -404,7 +404,7 @@ static void mem_info_la48(Monitor *mon, CPUArchState *env)
                     } else {
                         pd_addr = pdpe & 0x3fffffffff000ULL;
                         for (l3 = 0; l3 < 512; l3++) {
-                            cpu_physical_memory_read(pd_addr + l3 * 8, &pde, 8);
+                            cpu_physical_memory_read_debug(pd_addr + l3 * 8, &pde, 8);
                             pde = le64_to_cpu(pde);
                             end = (l1 << 39) + (l2 << 30) + (l3 << 21);
                             if (pde & PG_PRESENT_MASK) {
@@ -417,9 +417,9 @@ static void mem_info_la48(Monitor *mon, CPUArchState *env)
                                 } else {
                                     pt_addr = pde & 0x3fffffffff000ULL;
                                     for (l4 = 0; l4 < 512; l4++) {
-                                        cpu_physical_memory_read(pt_addr
-                                                                 + l4 * 8,
-                                                                 &pte, 8);
+                                        cpu_physical_memory_read_debug(pt_addr
+                                                                       + l4 * 8,
+                                                                       &pte, 8);
                                         pte = le64_to_cpu(pte);
                                         end = (l1 << 39) + (l2 << 30) +
                                             (l3 << 21) + (l4 << 12);
@@ -466,7 +466,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState *env)
     last_prot = 0;
     start = -1;
     for (l0 = 0; l0 < 512; l0++) {
-        cpu_physical_memory_read(pml5_addr + l0 * 8, &pml5e, 8);
+        cpu_physical_memory_read_debug(pml5_addr + l0 * 8, &pml5e, 8);
         pml5e = le64_to_cpu(pml5e);
         end = l0 << 48;
         if (!(pml5e & PG_PRESENT_MASK)) {
@@ -477,7 +477,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState *env)
 
         pml4_addr = pml5e & 0x3fffffffff000ULL;
         for (l1 = 0; l1 < 512; l1++) {
-            cpu_physical_memory_read(pml4_addr + l1 * 8, &pml4e, 8);
+            cpu_physical_memory_read_debug(pml4_addr + l1 * 8, &pml4e, 8);
             pml4e = le64_to_cpu(pml4e);
             end = (l0 << 48) + (l1 << 39);
             if (!(pml4e & PG_PRESENT_MASK)) {
@@ -488,7 +488,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState *env)
 
             pdp_addr = pml4e & 0x3fffffffff000ULL;
             for (l2 = 0; l2 < 512; l2++) {
-                cpu_physical_memory_read(pdp_addr + l2 * 8, &pdpe, 8);
+                cpu_physical_memory_read_debug(pdp_addr + l2 * 8, &pdpe, 8);
                 pdpe = le64_to_cpu(pdpe);
                 end = (l0 << 48) + (l1 << 39) + (l2 << 30);
                 if (pdpe & PG_PRESENT_MASK) {
@@ -507,7 +507,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState *env)
 
                 pd_addr = pdpe & 0x3fffffffff000ULL;
                 for (l3 = 0; l3 < 512; l3++) {
-                    cpu_physical_memory_read(pd_addr + l3 * 8, &pde, 8);
+                    cpu_physical_memory_read_debug(pd_addr + l3 * 8, &pde, 8);
                     pde = le64_to_cpu(pde);
                     end = (l0 << 48) + (l1 << 39) + (l2 << 30) + (l3 << 21);
                     if (pde & PG_PRESENT_MASK) {
@@ -526,7 +526,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState *env)
 
                     pt_addr = pde & 0x3fffffffff000ULL;
                     for (l4 = 0; l4 < 512; l4++) {
-                        cpu_physical_memory_read(pt_addr + l4 * 8, &pte, 8);
+                        cpu_physical_memory_read_debug(pt_addr + l4 * 8, &pte, 8);
                         pte = le64_to_cpu(pte);
                         end = (l0 << 48) + (l1 << 39) + (l2 << 30) +
                             (l3 << 21) + (l4 << 12);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 10/10] Introduce new CPUClass::get_phys_page_attrs_debug implementation for encrypted guests
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (8 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 09/10] Change the monitor and other commands and gdbstub to use the debug API Yuan Yao
@ 2021-05-06  1:40 ` Yuan Yao
  2021-09-02 14:04 ` [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Ashish Kalra
  10 siblings, 0 replies; 14+ messages in thread
From: Yuan Yao @ 2021-05-06  1:40 UTC (permalink / raw)
  To: pbonzini
  Cc: qemu-devel, kvm, dgilbert, ehabkost, mst, armbru, mtosatti,
	ashish.kalra, Thomas.Lendacky, brijesh.singh, isaku.yamahata,
	yuan.yao

From: Yuan Yao <yuan.yao@intel.com>

Add new function x86_cpu_get_phys_page_attrs_encrypted_debug() to walking guset
page tables to do VA -> PA translation for encrypted guests.

Now install this to cc->get_phys_page_attrs_debug for INTEL TD guests only.

Signed-off-by: Yuan Yao <yuan.yao@intel.com>

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 7a8a1386fb..9ce81bb21c 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1807,6 +1807,8 @@ void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags);
 
 hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
                                          MemTxAttrs *attrs);
+hwaddr x86_cpu_get_phys_page_attrs_encrypted_debug(CPUState *cs, vaddr addr,
+                                                   MemTxAttrs *attrs);
 
 int x86_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
 int x86_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target/i386/helper.c b/target/i386/helper.c
index 21edcb9204..a9a0467b50 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -205,8 +205,10 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
-                                         MemTxAttrs *attrs)
+static hwaddr x86_cpu_get_phys_page_attrs_debug_internal(CPUState *cs, vaddr addr,
+                                                         MemTxAttrs *attrs,
+                                                         uint64_t (*ldq_phys)(CPUState *, hwaddr),
+                                                         uint32_t (*ldl_phys)(CPUState *, hwaddr))
 {
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
@@ -242,7 +244,7 @@ hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
             if (la57) {
                 pml5e_addr = ((env->cr[3] & ~0xfff) +
                         (((addr >> 48) & 0x1ff) << 3)) & a20_mask;
-                pml5e = x86_ldq_phys(cs, pml5e_addr);
+                pml5e = ldq_phys(cs, pml5e_addr);
                 if (!(pml5e & PG_PRESENT_MASK)) {
                     return -1;
                 }
@@ -252,13 +254,13 @@ hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
 
             pml4e_addr = ((pml5e & PG_ADDRESS_MASK) +
                     (((addr >> 39) & 0x1ff) << 3)) & a20_mask;
-            pml4e = x86_ldq_phys(cs, pml4e_addr);
+            pml4e = ldq_phys(cs, pml4e_addr);
             if (!(pml4e & PG_PRESENT_MASK)) {
                 return -1;
             }
             pdpe_addr = ((pml4e & PG_ADDRESS_MASK) +
                          (((addr >> 30) & 0x1ff) << 3)) & a20_mask;
-            pdpe = x86_ldq_phys(cs, pdpe_addr);
+            pdpe = ldq_phys(cs, pdpe_addr);
             if (!(pdpe & PG_PRESENT_MASK)) {
                 return -1;
             }
@@ -273,14 +275,14 @@ hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
         {
             pdpe_addr = ((env->cr[3] & ~0x1f) + ((addr >> 27) & 0x18)) &
                 a20_mask;
-            pdpe = x86_ldq_phys(cs, pdpe_addr);
+            pdpe = ldq_phys(cs, pdpe_addr);
             if (!(pdpe & PG_PRESENT_MASK))
                 return -1;
         }
 
         pde_addr = ((pdpe & PG_ADDRESS_MASK) +
                     (((addr >> 21) & 0x1ff) << 3)) & a20_mask;
-        pde = x86_ldq_phys(cs, pde_addr);
+        pde = ldq_phys(cs, pde_addr);
         if (!(pde & PG_PRESENT_MASK)) {
             return -1;
         }
@@ -293,7 +295,7 @@ hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
             pte_addr = ((pde & PG_ADDRESS_MASK) +
                         (((addr >> 12) & 0x1ff) << 3)) & a20_mask;
             page_size = 4096;
-            pte = x86_ldq_phys(cs, pte_addr);
+            pte = ldq_phys(cs, pte_addr);
         }
         if (!(pte & PG_PRESENT_MASK)) {
             return -1;
@@ -303,7 +305,7 @@ hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
 
         /* page directory entry */
         pde_addr = ((env->cr[3] & ~0xfff) + ((addr >> 20) & 0xffc)) & a20_mask;
-        pde = x86_ldl_phys(cs, pde_addr);
+        pde = ldl_phys(cs, pde_addr);
         if (!(pde & PG_PRESENT_MASK))
             return -1;
         if ((pde & PG_PSE_MASK) && (env->cr[4] & CR4_PSE_MASK)) {
@@ -312,7 +314,7 @@ hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
         } else {
             /* page directory entry */
             pte_addr = ((pde & ~0xfff) + ((addr >> 10) & 0xffc)) & a20_mask;
-            pte = x86_ldl_phys(cs, pte_addr);
+            pte = ldl_phys(cs, pte_addr);
             if (!(pte & PG_PRESENT_MASK)) {
                 return -1;
             }
@@ -329,6 +331,22 @@ out:
     return pte | page_offset;
 }
 
+hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
+                                         MemTxAttrs *attrs)
+{
+    return x86_cpu_get_phys_page_attrs_debug_internal(cs, addr, attrs,
+                                                      x86_ldq_phys,
+                                                      x86_ldl_phys);
+}
+
+hwaddr x86_cpu_get_phys_page_attrs_encrypted_debug(CPUState *cs, vaddr addr,
+                                                   MemTxAttrs *attrs)
+{
+    return x86_cpu_get_phys_page_attrs_debug_internal(cs, addr, attrs,
+                                                      x86_ldq_phys_debug,
+                                                      x86_ldl_phys_debug);
+}
+
 typedef struct MCEInjectionParams {
     Monitor *mon;
     int bank;
diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c
index d13d4c8487..b1e089f73f 100644
--- a/target/i386/kvm/tdx.c
+++ b/target/i386/kvm/tdx.c
@@ -273,6 +273,7 @@ void tdx_pre_create_vcpu(CPUState *cpu)
 
     MachineState *ms = MACHINE(qdev_get_machine());
     X86CPU *x86cpu = X86_CPU(cpu);
+    CPUClass *cc = CPU_GET_CLASS(cpu);
     CPUX86State *env = &x86cpu->env;
     TdxGuest *tdx = (TdxGuest *)object_dynamic_cast(OBJECT(ms->cgs),
                                                     TYPE_TDX_GUEST);
@@ -320,6 +321,11 @@ void tdx_pre_create_vcpu(CPUState *cpu)
 
     init_vm.cpuid = (__u64)(&cpuid_data);
     tdx_ioctl(KVM_TDX_INIT_VM, 0, &init_vm);
+
+    if (tdx->debug) {
+        cc->get_phys_page_attrs_debug
+            = x86_cpu_get_phys_page_attrs_encrypted_debug;
+    }
 out:
     qemu_mutex_unlock(&tdx->lock);
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU
  2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
                   ` (9 preceding siblings ...)
  2021-05-06  1:40 ` [RFC][PATCH v1 10/10] Introduce new CPUClass::get_phys_page_attrs_debug implementation for encrypted guests Yuan Yao
@ 2021-09-02 14:04 ` Ashish Kalra
  2021-09-02 23:23   ` Yao, Yuan
  10 siblings, 1 reply; 14+ messages in thread
From: Ashish Kalra @ 2021-09-02 14:04 UTC (permalink / raw)
  To: yuan.yao
  Cc: Thomas.Lendacky, armbru, ashish.kalra, brijesh.singh, dgilbert,
	ehabkost, isaku.yamahata, kvm, mst, mtosatti, pbonzini,
	qemu-devel, yuan.yao

> - We introduce another new vm level ioctl focus on the encrypted
>     guest memory accessing:
>
>     KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY
>
>     struct kvm_rw_memory rw;
>     rw.addr = gpa_OR_hva;
>     rw.buf = (__u64)src;
>     rw.len = len;
>     kvm_vm_ioctl(kvm_state,
>                  KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY,
>                  &rw);
>
>     This new ioctl has more neutral and general name for its
>     purpose, the debugging support of AMD SEV and INTEL TDX
>     can be covered by a unify QEMU implementation on x86 with this
>     ioctl. Although only INTEL TD guest is supported in this series,
>     AMD SEV could be also supported with implementation of this
>     ioctl in KVM, plus small modifications in QEMU to enable the
>     unify part.

A general comment, we have sev_ioctl() interface for SEV guests and
probably this new vm level ioctl will not work for us.

It probably makes more sense to do this TDX/SEV level abstraction 
using the Memory Region's ram_debug_ops, which can point these to 
TDX specific vm level ioctl and SEV specific ioctl at the lowest
level of this interface.

Thanks,
Ashish

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU
  2021-09-02 14:04 ` [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Ashish Kalra
@ 2021-09-02 23:23   ` Yao, Yuan
  2021-09-07 10:51     ` Ashish Kalra
  0 siblings, 1 reply; 14+ messages in thread
From: Yao, Yuan @ 2021-09-02 23:23 UTC (permalink / raw)
  To: Ashish Kalra, yuan.yao
  Cc: Thomas.Lendacky, armbru, brijesh.singh, dgilbert, ehabkost,
	Yamahata, Isaku, kvm, mst, mtosatti, pbonzini, qemu-devel

>-----Original Message-----
>From: Ashish Kalra <Ashish.Kalra@amd.com>
>Sent: Thursday, September 02, 2021 22:05
>To: yuan.yao@linux.intel.com
>Cc: Thomas.Lendacky@amd.com; armbru@redhat.com; ashish.kalra@amd.com; brijesh.singh@amd.com;
>dgilbert@redhat.com; ehabkost@redhat.com; Yamahata, Isaku <isaku.yamahata@intel.com>; kvm@vger.kernel.org;
>mst@redhat.com; mtosatti@redhat.com; pbonzini@redhat.com; qemu-devel@nongnu.org; Yao, Yuan
><yuan.yao@intel.com>
>Subject: [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU
>
>> - We introduce another new vm level ioctl focus on the encrypted
>>     guest memory accessing:
>>
>>     KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY
>>
>>     struct kvm_rw_memory rw;
>>     rw.addr = gpa_OR_hva;
>>     rw.buf = (__u64)src;
>>     rw.len = len;
>>     kvm_vm_ioctl(kvm_state,
>>                  KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY,
>>                  &rw);
>>
>>     This new ioctl has more neutral and general name for its
>>     purpose, the debugging support of AMD SEV and INTEL TDX
>>     can be covered by a unify QEMU implementation on x86 with this
>>     ioctl. Although only INTEL TD guest is supported in this series,
>>     AMD SEV could be also supported with implementation of this
>>     ioctl in KVM, plus small modifications in QEMU to enable the
>>     unify part.
>
>A general comment, we have sev_ioctl() interface for SEV guests and
>probably this new vm level ioctl will not work for us.
>
>It probably makes more sense to do this TDX/SEV level abstraction
>using the Memory Region's ram_debug_ops, which can point these to
>TDX specific vm level ioctl and SEV specific ioctl at the lowest
>level of this interface.
>
Hi Ashish,

Yes, this new ioctl is now working as the low-level interface for 
Memory Region's ram_debug_ops. SEV can use 
kvm_setup_set_memory_region_debug_ops() to install a new
callback to KVM for installing SEV only low-level implementation,
then call kvm_set_memory_region_debug_ops() to do Memory
Region's ram_debug_ops installation later.


>Thanks,
>Ashish

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU
  2021-09-02 23:23   ` Yao, Yuan
@ 2021-09-07 10:51     ` Ashish Kalra
  0 siblings, 0 replies; 14+ messages in thread
From: Ashish Kalra @ 2021-09-07 10:51 UTC (permalink / raw)
  To: Yao, Yuan
  Cc: yuan.yao, Thomas.Lendacky, armbru, brijesh.singh, dgilbert,
	ehabkost, Yamahata, Isaku, kvm, mst, mtosatti, pbonzini,
	qemu-devel

Hello Yuan,

On Thu, Sep 02, 2021 at 11:23:50PM +0000, Yao, Yuan wrote:
> >-----Original Message-----
> >From: Ashish Kalra <Ashish.Kalra@amd.com>
> >Sent: Thursday, September 02, 2021 22:05
> >To: yuan.yao@linux.intel.com
> >Cc: Thomas.Lendacky@amd.com; armbru@redhat.com; ashish.kalra@amd.com; brijesh.singh@amd.com;
> >dgilbert@redhat.com; ehabkost@redhat.com; Yamahata, Isaku <isaku.yamahata@intel.com>; kvm@vger.kernel.org;
> >mst@redhat.com; mtosatti@redhat.com; pbonzini@redhat.com; qemu-devel@nongnu.org; Yao, Yuan
> ><yuan.yao@intel.com>
> >Subject: [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU
> >
> >> - We introduce another new vm level ioctl focus on the encrypted
> >>     guest memory accessing:
> >>
> >>     KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY
> >>
> >>     struct kvm_rw_memory rw;
> >>     rw.addr = gpa_OR_hva;
> >>     rw.buf = (__u64)src;
> >>     rw.len = len;
> >>     kvm_vm_ioctl(kvm_state,
> >>                  KVM_MEMORY_ENCRYPT_{READ,WRITE}_MEMORY,
> >>                  &rw);
> >>
> >>     This new ioctl has more neutral and general name for its
> >>     purpose, the debugging support of AMD SEV and INTEL TDX
> >>     can be covered by a unify QEMU implementation on x86 with this
> >>     ioctl. Although only INTEL TD guest is supported in this series,
> >>     AMD SEV could be also supported with implementation of this
> >>     ioctl in KVM, plus small modifications in QEMU to enable the
> >>     unify part.
> >
> >A general comment, we have sev_ioctl() interface for SEV guests and
> >probably this new vm level ioctl will not work for us.
> >
> >It probably makes more sense to do this TDX/SEV level abstraction
> >using the Memory Region's ram_debug_ops, which can point these to
> >TDX specific vm level ioctl and SEV specific ioctl at the lowest
> >level of this interface.
> >
> Hi Ashish,
> 
> Yes, this new ioctl is now working as the low-level interface for 
> Memory Region's ram_debug_ops. SEV can use 
> kvm_setup_set_memory_region_debug_ops() to install a new
> callback to KVM for installing SEV only low-level implementation,
> then call kvm_set_memory_region_debug_ops() to do Memory
> Region's ram_debug_ops installation later.
> 
> 

Ok. Yes i think that should work. 

Thanks,
Ashish

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-09-07 10:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-06  1:40 [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 01/10] Extend the MemTxAttrs to include a 'debug' flag. The flag can be used as general indicator that operation was triggered by the debugger Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 02/10] Currently, guest memory access for debugging purposes is performed using memcpy(). Extend the 'struct MemoryRegion' to include new callbacks that can be used to override the use of memcpy() with something else Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 03/10] Introduce new interface KVMState::set_mr_debug_ops and its wrapper Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 04/10] Implements the common MemoryRegion::ram_debug_ops for encrypted guests Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 05/10] Set the RAM's MemoryRegion::debug_ops for INTEL TD guests Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 06/10] Introduce new MemoryDebugOps which hook into guest virtual and physical memory debug interfaces such as cpu_memory_rw_debug, to allow vendor specific assist/hooks for debugging and delegating accessing the guest memory. This is required for example in case of AMD SEV platform where the guest memory is encrypted and a SEV specific debug assist/hook will be required to access the guest memory Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 07/10] Add new address_space_read and address_space_write debug helper interfaces which can be invoked by vendor specific guest memory debug assist/hooks to do guest RAM memory accesses using the added MemoryRegion callbacks Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 08/10] Introduce debug version of physical memory read/write API Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 09/10] Change the monitor and other commands and gdbstub to use the debug API Yuan Yao
2021-05-06  1:40 ` [RFC][PATCH v1 10/10] Introduce new CPUClass::get_phys_page_attrs_debug implementation for encrypted guests Yuan Yao
2021-09-02 14:04 ` [RFC][PATCH v1 00/10] Enable encrypted guest memory access in QEMU Ashish Kalra
2021-09-02 23:23   ` Yao, Yuan
2021-09-07 10:51     ` Ashish Kalra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).