sim: bfin: push down mmr address/size checks

The bfin port is using the WITH_DEVICES framework for two reasons:
- get access to the cpu making the request (if available)
- check the alignment & size for core & system MMRs

We addressed the first part with commit dea10706e9159ba6e94eab4c25010f3,
and we handle the second part with this commit.  Arguably this is more
correct too because trying to do bad reads/writes directly (when devices
support is disabled) often results in bad memory accesses.

As part of this clean up, we also adjust all of the existing logic that
would reject invalid accesses: the code was relying on the checks never
returning, but that's not the case when things like gdb (via the user's
commands) are making the requests.  Thus we'd still end up with bad mem
accesses, or sometimes gdb being hung due to while(1) loops.

Now we can connect (most of) these models into any address and have them
work correctly.

1 files changed, 27 insertions, 83 deletions

diff --git a/sim/bfin/devices.c b/sim/bfin/devices.c
index 7dab5f1..aec27d6 100644
--- a/sim/bfin/devices.c
+++ b/sim/bfin/devices.c
@@ -28,25 +28,32 @@
 #include "dv-bfin_mmu.h"
 
 static void
-bfin_mmr_invalid (struct hw *me, SIM_CPU *cpu, address_word addr,
-		  unsigned nr_bytes, bool write)
+bfin_mmr_invalid (struct hw *me, address_word addr,
+		  unsigned nr_bytes, bool write, bool missing)
 {
-  if (!cpu)
-    cpu = hw_system_cpu (me);
+  SIM_CPU *cpu = hw_system_cpu (me);
+  const char *rw = write ? "write" : "read";
+  const char *reason =
+    missing ? "no such register" :
+              (addr & 3) ? "must be 32-bit aligned" : "invalid length";
 
   /* Only throw a fit if the cpu is doing the access.  DMA/GDB simply
      go unnoticed.  Not exactly hardware behavior, but close enough.  */
   if (!cpu)
     {
-      sim_io_eprintf (hw_system (me), "%s: invalid MMR access @ %#x\n",
-		      hw_path (me), addr);
+      sim_io_eprintf (hw_system (me),
+		      "%s: invalid MMR %s at %#x length %u: %s\n",
+		      hw_path (me), rw, addr, nr_bytes, reason);
       return;
     }
 
-  HW_TRACE ((me, "invalid MMR %s to 0x%08lx length %u",
-	     write ? "write" : "read", (unsigned long) addr, nr_bytes));
+  HW_TRACE ((me, "invalid MMR %s at %#x length %u: %s",
+	     rw, addr, nr_bytes, reason));
 
-  /* XXX: is this what hardware does ?  */
+  /* XXX: is this what hardware does ?  What about priority of unaligned vs
+     wrong length vs missing register ?  What about system-vs-core ?  */
+  /* XXX: We should move this addr check to a model property so we get the
+     same behavior regardless of where we map the model.  */
   if (addr >= BFIN_CORE_MMR_BASE)
     /* XXX: This should be setting up CPLB fault addrs ?  */
     mmu_process_fault (cpu, addr, write, false, false, true);
@@ -60,93 +67,30 @@ void
 dv_bfin_mmr_invalid (struct hw *me, address_word addr, unsigned nr_bytes,
 		     bool write)
 {
-  bfin_mmr_invalid (me, NULL, addr, nr_bytes, write);
+  bfin_mmr_invalid (me, addr, nr_bytes, write, true);
 }
 
-void
+bool
 dv_bfin_mmr_require (struct hw *me, address_word addr, unsigned nr_bytes,
 		     unsigned size, bool write)
 {
-  if (nr_bytes != size)
-    dv_bfin_mmr_invalid (me, addr, nr_bytes, write);
-}
-
-static bool
-bfin_mmr_check (struct hw *me, SIM_CPU *cpu, address_word addr,
-		unsigned nr_bytes, bool write)
-{
-  if (addr >= BFIN_CORE_MMR_BASE)
-    {
-      /* All Core MMRs are aligned 32bits.  */
-      if ((addr & 3) == 0 && nr_bytes == 4)
-	return true;
-    }
-  else if (addr >= BFIN_SYSTEM_MMR_BASE)
-    {
-      /* All System MMRs are 32bit aligned, but can be 16bits or 32bits.  */
-      if ((addr & 0x3) == 0 && (nr_bytes == 2 || nr_bytes == 4))
-	return true;
-    }
-  else
+  if ((addr & 0x3) == 0 && nr_bytes == size)
     return true;
 
-  /* Still here ?  Must be crap.  */
-  bfin_mmr_invalid (me, cpu, addr, nr_bytes, write);
-
+  bfin_mmr_invalid (me, addr, nr_bytes, write, false);
   return false;
 }
 
+/* For 32-bit memory mapped registers that allow 16-bit or 32-bit access.  */
 bool
-dv_bfin_mmr_check (struct hw *me, address_word addr, unsigned nr_bytes,
-		   bool write)
-{
-  return bfin_mmr_check (me, NULL, addr, nr_bytes, write);
-}
-
-int
-device_io_read_buffer (device *me, void *source, int space,
-		       address_word addr, unsigned nr_bytes,
-		       SIM_DESC sd, SIM_CPU *cpu, sim_cia cia)
+dv_bfin_mmr_require_16_32 (struct hw *me, address_word addr, unsigned nr_bytes,
+			   bool write)
 {
-  struct hw *dv_me = (struct hw *) me;
-
-  if (STATE_ENVIRONMENT (sd) != OPERATING_ENVIRONMENT)
-    return nr_bytes;
-
-  if (bfin_mmr_check (dv_me, cpu, addr, nr_bytes, false))
-    if (cpu)
-      {
-	sim_cpu_hw_io_read_buffer (cpu, cia, dv_me, source, space,
-				   addr, nr_bytes);
-	return nr_bytes;
-      }
-    else
-      return sim_hw_io_read_buffer (sd, dv_me, source, space, addr, nr_bytes);
-  else
-    return 0;
-}
+  if ((addr & 0x3) == 0 && (nr_bytes == 2 || nr_bytes == 4))
+    return true;
 
-int
-device_io_write_buffer (device *me, const void *source, int space,
-			address_word addr, unsigned nr_bytes,
-                        SIM_DESC sd, SIM_CPU *cpu, sim_cia cia)
-{
-  struct hw *dv_me = (struct hw *) me;
-
-  if (STATE_ENVIRONMENT (sd) != OPERATING_ENVIRONMENT)
-    return nr_bytes;
-
-  if (bfin_mmr_check (dv_me, cpu, addr, nr_bytes, true))
-    if (cpu)
-      {
-	sim_cpu_hw_io_write_buffer (cpu, cia, dv_me, source, space,
-				    addr, nr_bytes);
-	return nr_bytes;
-      }
-    else
-      return sim_hw_io_write_buffer (sd, dv_me, source, space, addr, nr_bytes);
-  else
-    return 0;
+  bfin_mmr_invalid (me, addr, nr_bytes, write, false);
+  return false;
 }
 
 unsigned int dv_get_bus_num (struct hw *me)