1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] Fix wrong behavior of cpu_memory_rw_debug() function in SMM

Dmitry Poletaev posted 1 patch 6 years, 1 month ago
Test FreeBSD passed
Test docker-mingw@fedora passed
Test asan passed
Test docker-clang@ubuntu failed
Test checkpatch passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190918100706.19753-1-poletaev@ispras.ru
Maintainers: Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
target/i386/cpu.c    | 2 +-
target/i386/cpu.h    | 3 ++-
target/i386/helper.c | 5 ++++-
3 files changed, 7 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] Fix wrong behavior of cpu_memory_rw_debug() function in SMM
Posted by Dmitry Poletaev 6 years, 1 month ago 
There is a problem, that you don't have access to the data using cpu_memory_rw_debug() function when in SMM. You can't remotely debug SMM mode program because of that for example.
Likely attrs version of get_phys_page_debug should be used to get correct asidx at the end to handle access properly.
Here the patch to fix it.

Signed-off-by: Dmitry Poletaev <poletaev@ispras.ru>
---
 target/i386/cpu.c    | 2 +-
 target/i386/cpu.h    | 3 ++-
 target/i386/helper.c | 5 ++++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9e0bac31e8..8ade4ed2c6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5984,7 +5984,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 #ifndef CONFIG_USER_ONLY
     cc->asidx_from_attrs = x86_asidx_from_attrs;
     cc->get_memory_mapping = x86_cpu_get_memory_mapping;
-    cc->get_phys_page_debug = x86_cpu_get_phys_page_debug;
+    cc->get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug;
     cc->write_elf64_note = x86_cpu_write_elf64_note;
     cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
     cc->write_elf32_note = x86_cpu_write_elf32_note;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5f6e3a029a..bbd00d8deb 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1549,7 +1549,8 @@ void x86_cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
 
 void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags);
 
-hwaddr x86_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
+hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
+                                         MemTxAttrs *attrs);
 
 int x86_cpu_gdb_read_register(CPUState *cpu, uint8_t *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 0fa51be646..c3a6e4fabe 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -715,7 +715,8 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
+hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
+                                         MemTxAttrs *attrs)
 {
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
@@ -725,6 +726,8 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     uint32_t page_offset;
     int page_size;
 
+    *attrs = cpu_get_mem_attrs(env);
+
     a20_mask = x86_get_a20_mask(env);
     if (!(env->cr[0] & CR0_PG_MASK)) {
         pte = addr & a20_mask;
-- 
2.11.0
Re: [PATCH] Fix wrong behavior of cpu_memory_rw_debug() function in SMM
Posted by Paolo Bonzini 6 years, 1 month ago 
On 18/09/19 12:07, Dmitry Poletaev wrote:
> There is a problem, that you don't have access to the data using cpu_memory_rw_debug() function when in SMM. You can't remotely debug SMM mode program because of that for example.
> Likely attrs version of get_phys_page_debug should be used to get correct asidx at the end to handle access properly.
> Here the patch to fix it.
> 
> Signed-off-by: Dmitry Poletaev <poletaev@ispras.ru>
> ---
>  target/i386/cpu.c    | 2 +-
>  target/i386/cpu.h    | 3 ++-
>  target/i386/helper.c | 5 ++++-
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 9e0bac31e8..8ade4ed2c6 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5984,7 +5984,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  #ifndef CONFIG_USER_ONLY
>      cc->asidx_from_attrs = x86_asidx_from_attrs;
>      cc->get_memory_mapping = x86_cpu_get_memory_mapping;
> -    cc->get_phys_page_debug = x86_cpu_get_phys_page_debug;
> +    cc->get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug;
>      cc->write_elf64_note = x86_cpu_write_elf64_note;
>      cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
>      cc->write_elf32_note = x86_cpu_write_elf32_note;
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 5f6e3a029a..bbd00d8deb 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1549,7 +1549,8 @@ void x86_cpu_get_memory_mapping(CPUState *cpu, MemoryMappingList *list,
>  
>  void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags);
>  
> -hwaddr x86_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> +hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
> +                                         MemTxAttrs *attrs);
>  
>  int x86_cpu_gdb_read_register(CPUState *cpu, uint8_t *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 0fa51be646..c3a6e4fabe 100644
> --- a/target/i386/helper.c
> +++ b/target/i386/helper.c
> @@ -715,7 +715,8 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> -hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> +hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
> +                                         MemTxAttrs *attrs)
>  {
>      X86CPU *cpu = X86_CPU(cs);
>      CPUX86State *env = &cpu->env;
> @@ -725,6 +726,8 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>      uint32_t page_offset;
>      int page_size;
>  
> +    *attrs = cpu_get_mem_attrs(env);
> +
>      a20_mask = x86_get_a20_mask(env);
>      if (!(env->cr[0] & CR0_PG_MASK)) {
>          pte = addr & a20_mask;
> 

Queued, thanks.

Paolo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.