Re: [Qemu-devel] [RFC PATCH v1 15/22] i386: sev: register RAM read/write ops for BIOS and PC.RAM region

[Brijesh Singh]

Hi Paolo,

 typedef struct {
@@ -3568,6 +3578,7 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
     int l;
     hwaddr phys_addr;
     target_ulong page;
+    int mode = is_write ? WRITE_DATA : READ_DATA;

     while (len > 0) {
         int asidx;
@@ -3583,14 +3594,9 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
         if (l > len)
             l = len;
         phys_addr += (addr & ~TARGET_PAGE_MASK);
-        if (is_write) {
-            cpu_physical_memory_write_rom(cpu->cpu_ases[asidx].as,
-                                          phys_addr, buf, l);
-        } else {
-            address_space_rw(cpu->cpu_ases[asidx].as, phys_addr,
-                             MEMTXATTRS_UNSPECIFIED,
-                             buf, l, 0);
-        }
+        cpu_physical_memory_rw_debug_internal(cpu->cpu_ases[asidx].as,
+                                              phys_addr, buf, l,
+                                              mode);
         len -= l;
         buf += l;
         addr += l;


How do you want me to handle passing debug attrs (MEMTXATTRS_DEBUG) when 
doing a page walk to locate the physical page for a given virtual address.

I see something like this happen when we read virtual address from gdb 
or monitor commands.

cpu_memory_rw_debug
  cpu_get_phys_page_attrs_debug
      x86_cpu_get_phys_page_debug
        x86_ldq_phys
          attr = get_mem_debug_attrs
          address_space_ldq

get_mem_debug_attrs, does not set the MAXATTRS_DEBUG so we end up doing 
a memcpy instead of SEV debug read's. I was thinking about these two 
simple solution

1) something like this

diff --git a/target-i386/helper.c b/target-i386/helper.c
index a9d8aef..6322265 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1379,13 +1379,22 @@ void x86_cpu_exec_exit(CPUState *cs)
 }

 #ifndef CONFIG_USER_ONLY
+static inline MemTxAttrs get_mem_debug_attrs(CPUX86State *env)
+{
+    MemTxAttrs attrs = cpu_get_mem_attrs(env);
+
+    attrs.debug = MEMTXATTRS_DEBUG;
+
+    return attrs;
+}
+
 uint8_t x86_ldub_phys(CPUState *cs, hwaddr addr)
 {
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;

     return address_space_ldub(cs->as, addr,
-                              cpu_get_mem_attrs(env),
+                              get_mem_debug_attrs(env),
                               NULL);
 }

@@ -1395,7 +1404,7 @@ uint32_t x86_lduw_phys(CPUState *cs, hwaddr addr)
     CPUX86State *env = &cpu->env;

     return address_space_lduw(cs->as, addr,
-                              cpu_get_mem_attrs(env),
+                              get_mem_debug_attrs(env),
                               NULL);
 }



2) or implement and register a x86_cpu_get_phys_page_attrs_debug which 
takes care of setting the debug attribute before calling into 
address_space_ldq.

Please let me know your thought.

- Brijesh