All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code
@ 2018-11-22 17:26 Peter Maydell
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 1/3] disas.c: Use address_space_read() to read memory Peter Maydell
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Peter Maydell @ 2018-11-22 17:26 UTC (permalink / raw
  To: qemu-devel
  Cc: patches, Dr. David Alan Gilbert, Markus Armbruster,
	Philippe Mathieu-Daudé

This patchset takes three places in generic code which
use cpu_physical_memory_read(), and changes them to use
address_space_read() instead.

Changes v1->v2:
 * patches 1, 2 unchanged (and reviewed)
 * patch 3: handle as being NULL (which for the
   load_elf APIs means "use address_space_memory")

cpu_physical_memory_{read,rw,write} all implicitly assume
that there is exactly one view of physical memory. This
is sort-of true today, but we'd like to be able to move
to having heterogenous systems where not all CPUs share
the same view of physical memory.

In disas.c we are disassembling for a particular CPU, so
use that CPU's primary address space (cs->as).

In monitor.c we are reading physical memory for a
particular CPU, so again use that CPU's primary address
space; we fall back to address_space_memory for the case
where there are no CPUs in the system (-machine none).

In elf_ops.h the function was passed an address space to
use, so just use it.

Other places in generic code that use these functions are:
 * dump.c -- the whole UI here seems to assume that there
   is only one view of memory and that is what is being dumped
 * cpu.c:qmp_pmemsave() -- again, the UI assumption is that
   there's only one view of memory
So I've left those alone.

NB: git grep command line for finding callsites:
 git grep '\<cpu_physical_memory_\(read\|write\|rw\)\>'

thanks
-- PMM


Peter Maydell (3):
  disas.c: Use address_space_read() to read memory
  monitor: Use address_space_read() to read memory
  elf_ops.h: Use address_space_write() to write memory

 include/hw/elf_ops.h | 4 +++-
 disas.c              | 5 ++++-
 monitor.c            | 8 +++++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.19.1

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

* [Qemu-devel] [PATCH v2 for-4.0 1/3] disas.c: Use address_space_read() to read memory
  2018-11-22 17:26 [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell
@ 2018-11-22 17:26 ` Peter Maydell
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 2/3] monitor: " Peter Maydell
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2018-11-22 17:26 UTC (permalink / raw
  To: qemu-devel
  Cc: patches, Dr. David Alan Gilbert, Markus Armbruster,
	Philippe Mathieu-Daudé

Currently disas.c reads physical memory using
cpu_physical_memory_read(). This effectively hard-codes
assuming that all CPUs have the same view of physical
memory. Switch to address_space_read() instead, which
lets us use the AddressSpace for the CPU we're
disassembling for.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 disas.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/disas.c b/disas.c
index 5325b7e6be6..f9c517b3588 100644
--- a/disas.c
+++ b/disas.c
@@ -588,7 +588,10 @@ static int
 physical_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
                      struct disassemble_info *info)
 {
-    cpu_physical_memory_read(memaddr, myaddr, length);
+    CPUDebug *s = container_of(info, CPUDebug, info);
+
+    address_space_read(s->cpu->as, memaddr, MEMTXATTRS_UNSPECIFIED,
+                       myaddr, length);
     return 0;
 }
 
-- 
2.19.1

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

* [Qemu-devel] [PATCH v2 for-4.0 2/3] monitor: Use address_space_read() to read memory
  2018-11-22 17:26 [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 1/3] disas.c: Use address_space_read() to read memory Peter Maydell
@ 2018-11-22 17:26 ` Peter Maydell
  2018-11-22 22:30   ` Philippe Mathieu-Daudé
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 3/3] elf_ops.h: Use address_space_write() to write memory Peter Maydell
  2018-12-14 11:30 ` [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell
  3 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2018-11-22 17:26 UTC (permalink / raw
  To: qemu-devel
  Cc: patches, Dr. David Alan Gilbert, Markus Armbruster,
	Philippe Mathieu-Daudé

Currently monitor.c reads physical memory using
cpu_physical_memory_read(). This effectively hard-codes
assuming that all CPUs have the same view of physical
memory. Switch to address_space_read() instead, which
lets us use the AddressSpace for the CPU we're
reading memory for (falling back to address_space_memory
if there is no CPU, as happens with the "none" board).
As a bonus, this allows us to detect failures to read memory.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 monitor.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index d39390c2f2f..b0e8f2c490a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1604,7 +1604,13 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
         if (l > line_size)
             l = line_size;
         if (is_physical) {
-            cpu_physical_memory_read(addr, buf, l);
+            AddressSpace *as = cs ? cs->as : &address_space_memory;
+            MemTxResult r = address_space_read(as, addr,
+                                               MEMTXATTRS_UNSPECIFIED, buf, l);
+            if (r != MEMTX_OK) {
+                monitor_printf(mon, " Cannot access memory\n");
+                break;
+            }
         } else {
             if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
                 monitor_printf(mon, " Cannot access memory\n");
-- 
2.19.1

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

* [Qemu-devel] [PATCH v2 for-4.0 3/3] elf_ops.h: Use address_space_write() to write memory
  2018-11-22 17:26 [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 1/3] disas.c: Use address_space_read() to read memory Peter Maydell
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 2/3] monitor: " Peter Maydell
@ 2018-11-22 17:26 ` Peter Maydell
  2018-11-22 22:28   ` Philippe Mathieu-Daudé
  2018-12-14 11:30 ` [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell
  3 siblings, 1 reply; 7+ messages in thread
From: Peter Maydell @ 2018-11-22 17:26 UTC (permalink / raw
  To: qemu-devel
  Cc: patches, Dr. David Alan Gilbert, Markus Armbruster,
	Philippe Mathieu-Daudé

Currently the load_elf function in elf_ops.h uses
cpu_physical_memory_write() to write the ELF file to
memory if it is not handling it as a ROM blob. This
means we ignore the AddressSpace that the function
is passed to define where it should be loaded.
Use address_space_write() instead.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
v1->v2: handle NULL as
---
 include/hw/elf_ops.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 81cecaf27e2..74679ff8da3 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -482,7 +482,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
                     rom_add_elf_program(label, data, file_size, mem_size,
                                         addr, as);
                 } else {
-                    cpu_physical_memory_write(addr, data, file_size);
+                    address_space_write(as ? as : &address_space_memory,
+                                        addr, MEMTXATTRS_UNSPECIFIED,
+                                        data, file_size);
                     g_free(data);
                 }
             }
-- 
2.19.1

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

* Re: [Qemu-devel] [PATCH v2 for-4.0 3/3] elf_ops.h: Use address_space_write() to write memory
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 3/3] elf_ops.h: Use address_space_write() to write memory Peter Maydell
@ 2018-11-22 22:28   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-22 22:28 UTC (permalink / raw
  To: Peter Maydell, qemu-devel
  Cc: Markus Armbruster, Dr. David Alan Gilbert, patches

On 22/11/18 18:26, Peter Maydell wrote:
> Currently the load_elf function in elf_ops.h uses
> cpu_physical_memory_write() to write the ELF file to
> memory if it is not handling it as a ROM blob. This
> means we ignore the AddressSpace that the function
> is passed to define where it should be loaded.
> Use address_space_write() instead.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> v1->v2: handle NULL as

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  include/hw/elf_ops.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 81cecaf27e2..74679ff8da3 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -482,7 +482,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>                      rom_add_elf_program(label, data, file_size, mem_size,
>                                          addr, as);
>                  } else {
> -                    cpu_physical_memory_write(addr, data, file_size);
> +                    address_space_write(as ? as : &address_space_memory,
> +                                        addr, MEMTXATTRS_UNSPECIFIED,
> +                                        data, file_size);
>                      g_free(data);
>                  }
>              }
> 

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

* Re: [Qemu-devel] [PATCH v2 for-4.0 2/3] monitor: Use address_space_read() to read memory
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 2/3] monitor: " Peter Maydell
@ 2018-11-22 22:30   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-22 22:30 UTC (permalink / raw
  To: Peter Maydell, qemu-devel
  Cc: Markus Armbruster, Dr. David Alan Gilbert, patches

On 22/11/18 18:26, Peter Maydell wrote:
> Currently monitor.c reads physical memory using
> cpu_physical_memory_read(). This effectively hard-codes
> assuming that all CPUs have the same view of physical
> memory. Switch to address_space_read() instead, which
> lets us use the AddressSpace for the CPU we're
> reading memory for (falling back to address_space_memory
> if there is no CPU, as happens with the "none" board).
> As a bonus, this allows us to detect failures to read memory.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  monitor.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/monitor.c b/monitor.c
> index d39390c2f2f..b0e8f2c490a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1604,7 +1604,13 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
>          if (l > line_size)
>              l = line_size;
>          if (is_physical) {
> -            cpu_physical_memory_read(addr, buf, l);
> +            AddressSpace *as = cs ? cs->as : &address_space_memory;
> +            MemTxResult r = address_space_read(as, addr,
> +                                               MEMTXATTRS_UNSPECIFIED, buf, l);
> +            if (r != MEMTX_OK) {
> +                monitor_printf(mon, " Cannot access memory\n");
> +                break;
> +            }
>          } else {
>              if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
>                  monitor_printf(mon, " Cannot access memory\n");
> 

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

* Re: [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code
  2018-11-22 17:26 [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell
                   ` (2 preceding siblings ...)
  2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 3/3] elf_ops.h: Use address_space_write() to write memory Peter Maydell
@ 2018-12-14 11:30 ` Peter Maydell
  3 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2018-12-14 11:30 UTC (permalink / raw
  To: QEMU Developers
  Cc: Markus Armbruster, Philippe Mathieu-Daudé,
	Dr. David Alan Gilbert, patches@linaro.org

On Thu, 22 Nov 2018 at 17:28, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset takes three places in generic code which
> use cpu_physical_memory_read(), and changes them to use
> address_space_read() instead.
>
> Changes v1->v2:
>  * patches 1, 2 unchanged (and reviewed)
>  * patch 3: handle as being NULL (which for the
>    load_elf APIs means "use address_space_memory")

I'm adding this to a 'misc' pullreq I'm putting together.

thanks
-- PMM

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

end of thread, other threads:[~2018-12-14 11:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-22 17:26 [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell
2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 1/3] disas.c: Use address_space_read() to read memory Peter Maydell
2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 2/3] monitor: " Peter Maydell
2018-11-22 22:30   ` Philippe Mathieu-Daudé
2018-11-22 17:26 ` [Qemu-devel] [PATCH v2 for-4.0 3/3] elf_ops.h: Use address_space_write() to write memory Peter Maydell
2018-11-22 22:28   ` Philippe Mathieu-Daudé
2018-12-14 11:30 ` [Qemu-devel] [PATCH v2 for-4.0 0/3] Avoid cpu_physical_memory_read() in generic code Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.