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