Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-07-16 Thread Jan Kiszka 
On 2013-05-30 23:03, Paolo Bonzini wrote:
 This provides the basics for detecting accesses to unassigned memory
 as soon as they happen, and also for a simple implementation of
 address_space_access_valid.
 
 Reviewed-by: Richard Henderson r...@twiddle.net
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  exec.c   |   36 
  memory.c |   28 ++--
  2 files changed, 38 insertions(+), 26 deletions(-)
 
 diff --git a/exec.c b/exec.c
 index 785eeeb..c5100d6 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
  return ram_addr;
  }
  
 -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
 -unsigned size)
 +static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
 +   unsigned size, bool is_write)
  {
 -#ifdef DEBUG_UNASSIGNED
 -printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
 -#endif
 -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 -cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
 -#endif
 -return 0;
 -}
 -
 -static void unassigned_mem_write(void *opaque, hwaddr addr,
 - uint64_t val, unsigned size)
 -{
 -#ifdef DEBUG_UNASSIGNED
 -printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
 val);
 -#endif
 -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 -cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
 -#endif
 +return false;
  }
  
 -static const MemoryRegionOps unassigned_mem_ops = {
 -.read = unassigned_mem_read,
 -.write = unassigned_mem_write,
 +const MemoryRegionOps unassigned_mem_ops = {
 +.valid.accepts = unassigned_mem_accepts,
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
 @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr 
 ram_addr,
  tlb_set_dirty(cpu_single_env, cpu_single_env-mem_io_vaddr);
  }
  
 +static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
 + unsigned size, bool is_write)
 +{
 +return is_write;
 +}
 +
  static const MemoryRegionOps notdirty_mem_ops = {
 -.read = unassigned_mem_read,
  .write = notdirty_mem_write,
 +.valid.accepts = notdirty_mem_accepts,
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
 diff --git a/memory.c b/memory.c
 index 99f046d..15da877 100644
 --- a/memory.c
 +++ b/memory.c
 @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
  mr-flush_coalesced_mmio = false;
  }
  
 +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
 +unsigned size)
 +{
 +#ifdef DEBUG_UNASSIGNED
 +printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
 +#endif
 +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 +cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
 +#endif
 +return 0;

This changed the value read from unassigned memory from -1 to 0. Any
particular reason or an unintentional change?

Note that this also breaks unassigned portio accesses, specifically the
case Stefan reported around IPMI access of the Linux kernel.

Jan

 +}
 +
 +static void unassigned_mem_write(void *opaque, hwaddr addr,
 + uint64_t val, unsigned size)
 +{
 +#ifdef DEBUG_UNASSIGNED
 +printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
 val);
 +#endif
 +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 +cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
 +#endif
 +}
 +
  static bool memory_region_access_valid(MemoryRegion *mr,
 hwaddr addr,
 unsigned size,
 @@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion 
 *mr,
  uint64_t data = 0;
  
  if (!memory_region_access_valid(mr, addr, size, false)) {
 -return -1U; /* FIXME: better signalling */
 +return unassigned_mem_read(mr, addr, size);
  }
  
  if (!mr-ops-read) {
 @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
   unsigned size)
  {
  if (!memory_region_access_valid(mr, addr, size, true)) {
 -return; /* FIXME: better signalling */
 +unassigned_mem_write(mr, addr, data, size);
 +return;
  }
  
  adjust_endianness(mr, data, size);
 
-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-07-16 Thread Paolo Bonzini 
Il 16/07/2013 14:28, Jan Kiszka ha scritto:
 On 2013-05-30 23:03, Paolo Bonzini wrote:
 This provides the basics for detecting accesses to unassigned memory
 as soon as they happen, and also for a simple implementation of
 address_space_access_valid.

 Reviewed-by: Richard Henderson r...@twiddle.net
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  exec.c   |   36 
  memory.c |   28 ++--
  2 files changed, 38 insertions(+), 26 deletions(-)

 diff --git a/exec.c b/exec.c
 index 785eeeb..c5100d6 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
  return ram_addr;
  }
  
 -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
 -unsigned size)
 +static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
 +   unsigned size, bool is_write)
  {
 -#ifdef DEBUG_UNASSIGNED
 -printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
 -#endif
 -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 -cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
 -#endif
 -return 0;
 -}
 -
 -static void unassigned_mem_write(void *opaque, hwaddr addr,
 - uint64_t val, unsigned size)
 -{
 -#ifdef DEBUG_UNASSIGNED
 -printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
 val);
 -#endif
 -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 -cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
 -#endif
 +return false;
  }
  
 -static const MemoryRegionOps unassigned_mem_ops = {
 -.read = unassigned_mem_read,
 -.write = unassigned_mem_write,
 +const MemoryRegionOps unassigned_mem_ops = {
 +.valid.accepts = unassigned_mem_accepts,
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
 @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr 
 ram_addr,
  tlb_set_dirty(cpu_single_env, cpu_single_env-mem_io_vaddr);
  }
  
 +static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
 + unsigned size, bool is_write)
 +{
 +return is_write;
 +}
 +
  static const MemoryRegionOps notdirty_mem_ops = {
 -.read = unassigned_mem_read,
  .write = notdirty_mem_write,
 +.valid.accepts = notdirty_mem_accepts,
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  
 diff --git a/memory.c b/memory.c
 index 99f046d..15da877 100644
 --- a/memory.c
 +++ b/memory.c
 @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
  mr-flush_coalesced_mmio = false;
  }
  
 +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
 +unsigned size)
 +{
 +#ifdef DEBUG_UNASSIGNED
 +printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
 +#endif
 +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 +cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
 +#endif
 +return 0;
 
 This changed the value read from unassigned memory from -1 to 0. Any
 particular reason or an unintentional change?

Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to
return -1, unifying the paths dropped the difference).

I guess unassigned RAM can read as -1 just fine, so we can just change
unassigned_mem_read to return -1.

Paolo

 Jan
 
 +}
 +
 +static void unassigned_mem_write(void *opaque, hwaddr addr,
 + uint64_t val, unsigned size)
 +{
 +#ifdef DEBUG_UNASSIGNED
 +printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
 val);
 +#endif
 +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 +cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
 +#endif
 +}
 +
  static bool memory_region_access_valid(MemoryRegion *mr,
 hwaddr addr,
 unsigned size,
 @@ -847,7 +870,7 @@ static uint64_t 
 memory_region_dispatch_read1(MemoryRegion *mr,
  uint64_t data = 0;
  
  if (!memory_region_access_valid(mr, addr, size, false)) {
 -return -1U; /* FIXME: better signalling */
 +return unassigned_mem_read(mr, addr, size);
  }
  
  if (!mr-ops-read) {
 @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion 
 *mr,
   unsigned size)
  {
  if (!memory_region_access_valid(mr, addr, size, true)) {
 -return; /* FIXME: better signalling */
 +unassigned_mem_write(mr, addr, data, size);
 +return;
  }
  
  adjust_endianness(mr, data, size);

Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-07-16 Thread Peter Maydell 
On 16 July 2013 13:33, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 16/07/2013 14:28, Jan Kiszka ha scritto:
 This changed the value read from unassigned memory from -1 to 0. Any
 particular reason or an unintentional change?

 Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to
 return -1, unifying the paths dropped the difference).

 I guess unassigned RAM can read as -1 just fine, so we can just change
 unassigned_mem_read to return -1.

Behaviour for accesses to unassigned addresses should really
be both bus and CPU specific...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-07-16 Thread Jan Kiszka 
On 2013-07-16 14:38, Peter Maydell wrote:
 On 16 July 2013 13:33, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 16/07/2013 14:28, Jan Kiszka ha scritto:
 This changed the value read from unassigned memory from -1 to 0. Any
 particular reason or an unintentional change?

 Cut-and-paste (unassigned RAM used to return 0, invalid MMIO used to
 return -1, unifying the paths dropped the difference).

 I guess unassigned RAM can read as -1 just fine, so we can just change
 unassigned_mem_read to return -1.
 
 Behaviour for accesses to unassigned addresses should really
 be both bus and CPU specific...

Yes, that's what I was thinking as well when reading it. Still, lets
restore what we did before, then refine.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-06-03 Thread Paolo Bonzini 
Il 01/06/2013 17:28, Blue Swirl ha scritto:
 This means that memory.c is getting knowledge about CPU types and it
 becomes more specific to current target. I think memory.c should be
 generic and target agnostic (maybe one day compiled just once) with
 exec.c implementing the target specific functionality.

You're right, but the solution is simply to make cpu_unassigned_access a
CPU method.

Paolo

Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-06-03 Thread Andreas Färber 
Am 03.06.2013 09:31, schrieb Paolo Bonzini:
 Il 01/06/2013 17:28, Blue Swirl ha scritto:
 This means that memory.c is getting knowledge about CPU types and it
 becomes more specific to current target. I think memory.c should be
 generic and target agnostic (maybe one day compiled just once) with
 exec.c implementing the target specific functionality.
 
 You're right, but the solution is simply to make cpu_unassigned_access a
 CPU method.

I already have a patch queued to that effect on qom-cpu-10 branch that I
needed to rebase after your merge. :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-06-01 Thread Blue Swirl 
On Thu, May 30, 2013 at 9:03 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 This provides the basics for detecting accesses to unassigned memory
 as soon as they happen, and also for a simple implementation of
 address_space_access_valid.

 Reviewed-by: Richard Henderson r...@twiddle.net
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  exec.c   |   36 
  memory.c |   28 ++--
  2 files changed, 38 insertions(+), 26 deletions(-)

 diff --git a/exec.c b/exec.c
 index 785eeeb..c5100d6 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
  return ram_addr;
  }

 -static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
 -unsigned size)
 +static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
 +   unsigned size, bool is_write)
  {
 -#ifdef DEBUG_UNASSIGNED
 -printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
 -#endif
 -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 -cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
 -#endif
 -return 0;
 -}
 -
 -static void unassigned_mem_write(void *opaque, hwaddr addr,
 - uint64_t val, unsigned size)
 -{
 -#ifdef DEBUG_UNASSIGNED
 -printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
 val);
 -#endif
 -#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 -cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
 -#endif
 +return false;
  }

 -static const MemoryRegionOps unassigned_mem_ops = {
 -.read = unassigned_mem_read,
 -.write = unassigned_mem_write,
 +const MemoryRegionOps unassigned_mem_ops = {
 +.valid.accepts = unassigned_mem_accepts,
  .endianness = DEVICE_NATIVE_ENDIAN,
  };

 @@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr 
 ram_addr,
  tlb_set_dirty(cpu_single_env, cpu_single_env-mem_io_vaddr);
  }

 +static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
 + unsigned size, bool is_write)
 +{
 +return is_write;
 +}
 +
  static const MemoryRegionOps notdirty_mem_ops = {
 -.read = unassigned_mem_read,
  .write = notdirty_mem_write,
 +.valid.accepts = notdirty_mem_accepts,
  .endianness = DEVICE_NATIVE_ENDIAN,
  };

 diff --git a/memory.c b/memory.c
 index 99f046d..15da877 100644
 --- a/memory.c
 +++ b/memory.c
 @@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
  mr-flush_coalesced_mmio = false;
  }

 +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
 +unsigned size)
 +{
 +#ifdef DEBUG_UNASSIGNED
 +printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
 +#endif
 +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 +cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);

This means that memory.c is getting knowledge about CPU types and it
becomes more specific to current target. I think memory.c should be
generic and target agnostic (maybe one day compiled just once) with
exec.c implementing the target specific functionality.

 +#endif
 +return 0;
 +}
 +
 +static void unassigned_mem_write(void *opaque, hwaddr addr,
 + uint64_t val, unsigned size)
 +{
 +#ifdef DEBUG_UNASSIGNED
 +printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
 val);
 +#endif
 +#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
 defined(TARGET_MICROBLAZE)
 +cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
 +#endif
 +}
 +
  static bool memory_region_access_valid(MemoryRegion *mr,
 hwaddr addr,
 unsigned size,
 @@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion 
 *mr,
  uint64_t data = 0;

  if (!memory_region_access_valid(mr, addr, size, false)) {
 -return -1U; /* FIXME: better signalling */
 +return unassigned_mem_read(mr, addr, size);
  }

  if (!mr-ops-read) {
 @@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
   unsigned size)
  {
  if (!memory_region_access_valid(mr, addr, size, true)) {
 -return; /* FIXME: better signalling */
 +unassigned_mem_write(mr, addr, data, size);
 +return;
  }

  adjust_endianness(mr, data, size);
 --
 1.7.4.1

[Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-05-30 Thread Paolo Bonzini 
This provides the basics for detecting accesses to unassigned memory
as soon as they happen, and also for a simple implementation of
address_space_access_valid.

Reviewed-by: Richard Henderson r...@twiddle.net
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 exec.c   |   36 
 memory.c |   28 ++--
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/exec.c b/exec.c
index 785eeeb..c5100d6 100644
--- a/exec.c
+++ b/exec.c
@@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
 return ram_addr;
 }
 
-static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-unsigned size)
+static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
+   unsigned size, bool is_write)
 {
-#ifdef DEBUG_UNASSIGNED
-printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
-cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
-#endif
-return 0;
-}
-
-static void unassigned_mem_write(void *opaque, hwaddr addr,
- uint64_t val, unsigned size)
-{
-#ifdef DEBUG_UNASSIGNED
-printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
val);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
-cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
-#endif
+return false;
 }
 
-static const MemoryRegionOps unassigned_mem_ops = {
-.read = unassigned_mem_read,
-.write = unassigned_mem_write,
+const MemoryRegionOps unassigned_mem_ops = {
+.valid.accepts = unassigned_mem_accepts,
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
@@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr 
ram_addr,
 tlb_set_dirty(cpu_single_env, cpu_single_env-mem_io_vaddr);
 }
 
+static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
+ unsigned size, bool is_write)
+{
+return is_write;
+}
+
 static const MemoryRegionOps notdirty_mem_ops = {
-.read = unassigned_mem_read,
 .write = notdirty_mem_write,
+.valid.accepts = notdirty_mem_accepts,
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
diff --git a/memory.c b/memory.c
index 99f046d..15da877 100644
--- a/memory.c
+++ b/memory.c
@@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
 mr-flush_coalesced_mmio = false;
 }
 
+static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
+unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
+cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
+#endif
+return 0;
+}
+
+static void unassigned_mem_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
val);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
+cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
+#endif
+}
+
 static bool memory_region_access_valid(MemoryRegion *mr,
hwaddr addr,
unsigned size,
@@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion 
*mr,
 uint64_t data = 0;
 
 if (!memory_region_access_valid(mr, addr, size, false)) {
-return -1U; /* FIXME: better signalling */
+return unassigned_mem_read(mr, addr, size);
 }
 
 if (!mr-ops-read) {
@@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
  unsigned size)
 {
 if (!memory_region_access_valid(mr, addr, size, true)) {
-return; /* FIXME: better signalling */
+unassigned_mem_write(mr, addr, data, size);
+return;
 }
 
 adjust_endianness(mr, data, size);
-- 
1.7.4.1

[Qemu-devel] [PATCH 06/22] memory: dispatch unassigned accesses based on .valid.accepts

2013-05-24 Thread Paolo Bonzini 
This provides the basics for detecting accesses to unassigned memory
as soon as they happen, and also for a simple implementation of
address_space_access_valid.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 exec.c   | 36 
 memory.c | 28 ++--
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/exec.c b/exec.c
index 785eeeb..c5100d6 100644
--- a/exec.c
+++ b/exec.c
@@ -50,7 +50,6 @@
 
 #include exec/memory-internal.h
 
-//#define DEBUG_UNASSIGNED
 //#define DEBUG_SUBPAGE
 
 #if !defined(CONFIG_USER_ONLY)
@@ -1383,32 +1383,14 @@ ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr)
 return ram_addr;
 }
 
-static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-unsigned size)
+static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
+   unsigned size, bool is_write)
 {
-#ifdef DEBUG_UNASSIGNED
-printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
-cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
-#endif
-return 0;
-}
-
-static void unassigned_mem_write(void *opaque, hwaddr addr,
- uint64_t val, unsigned size)
-{
-#ifdef DEBUG_UNASSIGNED
-printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
val);
-#endif
-#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
-cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
-#endif
+return false;
 }
 
-static const MemoryRegionOps unassigned_mem_ops = {
-.read = unassigned_mem_read,
-.write = unassigned_mem_write,
+const MemoryRegionOps unassigned_mem_ops = {
+.valid.accepts = unassigned_mem_accepts,
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
@@ -1442,9 +1424,15 @@ static void notdirty_mem_write(void *opaque, hwaddr 
ram_addr,
 tlb_set_dirty(cpu_single_env, cpu_single_env-mem_io_vaddr);
 }
 
+static bool notdirty_mem_accepts(void *opaque, hwaddr addr,
+ unsigned size, bool is_write)
+{
+return is_write;
+}
+
 static const MemoryRegionOps notdirty_mem_ops = {
-.read = unassigned_mem_read,
 .write = notdirty_mem_write,
+.valid.accepts = notdirty_mem_accepts,
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
diff --git a/memory.c b/memory.c
index 99f046d..15da877 100644
--- a/memory.c
+++ b/memory.c
@@ -22,6 +22,8 @@
 
 #include exec/memory-internal.h
 
+//#define DEBUG_UNASSIGNED
+
 static unsigned memory_region_transaction_depth;
 static bool memory_region_update_pending;
 static bool global_dirty_log = false;
@@ -814,6 +814,29 @@ void memory_region_init(MemoryRegion *mr,
 mr-flush_coalesced_mmio = false;
 }
 
+static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
+unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+printf(Unassigned mem read  TARGET_FMT_plx \n, addr);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
+cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, size);
+#endif
+return 0;
+}
+
+static void unassigned_mem_write(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+#ifdef DEBUG_UNASSIGNED
+printf(Unassigned mem write  TARGET_FMT_plx  = 0x%PRIx64\n, addr, 
val);
+#endif
+#if defined(TARGET_ALPHA) || defined(TARGET_SPARC) || 
defined(TARGET_MICROBLAZE)
+cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, size);
+#endif
+}
+
 static bool memory_region_access_valid(MemoryRegion *mr,
hwaddr addr,
unsigned size,
@@ -847,7 +870,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion 
*mr,
 uint64_t data = 0;
 
 if (!memory_region_access_valid(mr, addr, size, false)) {
-return -1U; /* FIXME: better signalling */
+return unassigned_mem_read(mr, addr, size);
 }
 
 if (!mr-ops-read) {
@@ -898,7 +921,8 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
  unsigned size)
 {
 if (!memory_region_access_valid(mr, addr, size, true)) {
-return; /* FIXME: better signalling */
+unassigned_mem_write(mr, addr, data, size);
+return;
 }
 
 adjust_endianness(mr, data, size);
-- 
1.8.1.4