On 26/10/2024 08:56, Thomas Huth wrote:

Am Wed, 23 Oct 2024 09:58:19 +0100
schrieb Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>:

Change the start of the next.mmio memory region so that it follows on directly
after the next.dma memory region, adjusting the address offsets in
next_mmio_read() and next_mmio_write() accordingly.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
  hw/m68k/next-cube.c | 28 ++++++++++++++--------------
  1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index 4e8e55a8bd..e1d94c1ce0 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -266,23 +266,23 @@ static uint64_t next_mmio_read(void *opaque, hwaddr addr, 
unsigned size)
      uint64_t val;
switch (addr) {
-    case 0x7000:
+    case 0x2000:
          /* DPRINTF("Read INT status: %x\n", s->int_status); */
          val = s->int_status;
          break;
- case 0x7800:
+    case 0x2800:
          DPRINTF("MMIO Read INT mask: %x\n", s->int_mask);
          val = s->int_mask;
          break;
- case 0xc000 ... 0xc003:
-        val = extract32(s->scr1, (4 - (addr - 0xc000) - size) << 3,
+    case 0x7000 ... 0x7003:
+        val = extract32(s->scr1, (4 - (addr - 0x7000) - size) << 3,
                          size << 3);
          break;
- case 0xd000 ... 0xd003:
-        val = extract32(s->scr2, (4 - (addr - 0xd000) - size) << 3,
+    case 0x8000 ... 0x8003:
+        val = extract32(s->scr2, (4 - (addr - 0x8000) - size) << 3,
                          size << 3);
          break;
@@ -301,25 +301,25 @@ static void next_mmio_write(void *opaque, hwaddr addr, uint64_t val,
      NeXTPC *s = NEXT_PC(opaque);
switch (addr) {
-    case 0x7000:
+    case 0x2000:
          DPRINTF("INT Status old: %x new: %x\n", s->int_status,
                  (unsigned int)val);
          s->int_status = val;
          break;
- case 0x7800:
+    case 0x2800:
          DPRINTF("INT Mask old: %x new: %x\n", s->int_mask, (unsigned int)val);
          s->int_mask  = val;
          break;

Could you please add comments at the end of the "case" lines, stating which
mmio addresses are handled in each case? Otherwise, it's harder to grep for
certain addresses later. E.g:

     case 0x2800:     /* 0x2007800 */

If you think it will help? Presumably this is to aid with comparing with other source code/documentation?

@@ -1000,7 +1000,7 @@ static void next_cube_init(MachineState *machine)
      sysbus_create_simple(TYPE_NEXTFB, 0x0B000000, NULL);
/* MMIO */
-    sysbus_mmio_map(SYS_BUS_DEVICE(pcdev), 0, 0x02000000);
+    sysbus_mmio_map(SYS_BUS_DEVICE(pcdev), 0, 0x02005000);

Why 0x02005000 and not directly 0x02007000 ?

Before this patch the output of "info mtree" shows as follows:

(qemu) info mtree
address-space: cpu-memory-0
address-space: memory
  0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000000001ffff (prio 0, rom): alias next.rom2 @next.rom 0000000000000000-000000000001ffff
    0000000001000000-000000000101ffff (prio 0, rom): next.rom
    0000000002000000-0000000002004fff (prio 0, i/o): next.dma
    0000000002000000-00000000020cffff (prio 0, i/o): next.mmio
    000000000200e000-000000000200efff (prio 0, i/o): next.kbd
    00000000020c0000-00000000020c003f (prio 0, ram): next.bmapmem
    0000000002100000-000000000211ffff (prio 0, i/o): next.scr
    0000000002114000-000000000211400f (prio 0, i/o): esp-regs
    0000000002118000-0000000002118003 (prio 0, i/o): escc
    0000000004000000-0000000007ffffff (prio 0, ram): next.ram
    000000000b000000-000000000b1cb0ff (prio 0, ram): next-video
00000000820c0000-00000000820c003f (prio 0, ram): alias next.bmapmem2 @next.bmapmem 0000000000000000-000000000000003f

All this patch does is move the start of next.mmio to 0x2005000 to avoid the 
overlap.

I think there is another range at 0x02006000 related to the ethernet
controller, so directly going with 0x02007000 might cause less friction
later when we add the NIC?

By the end of the series, everything except the "global" registers in next.mmio have their own memory region (or empty-slot if the target is unknown) so that "info mtree" output looks like this:

(qemu) info mtree
address-space: cpu-memory-0
address-space: memory
  0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-000000000001ffff (prio 0, rom): alias next.rom2 @next.rom 0000000000000000-000000000001ffff
    0000000001000000-000000000101ffff (prio 0, rom): next.rom
    0000000002000000-0000000002004fff (prio 0, i/o): next.dma
    0000000002005000-000000000200dfff (prio 0, i/o): next.mmio
    000000000200e000-000000000200efff (prio 0, i/o): next.kbd
    00000000020c0000-00000000020c003f (prio 0, ram): next.bmapmem
    0000000002106000-000000000210601f (prio 0, i/o): next.en
    0000000002110000-000000000211000f (prio -10000, i/o): empty-slot
    0000000002112000-000000000211200f (prio -10000, i/o): empty-slot
    0000000002114000-000000000211403f (prio 0, i/o): next.scsi
      0000000002114000-000000000211400f (prio 0, i/o): esp-regs
      0000000002114020-0000000002114021 (prio 0, i/o): csrs
    0000000002114108-000000000211410b (prio 0, i/o): next.floppy
    0000000002118000-0000000002118003 (prio 0, i/o): escc
    0000000002118004-0000000002118013 (prio -10000, i/o): empty-slot
    000000000211a000-000000000211a003 (prio 0, i/o): next.timer
    0000000004000000-0000000007ffffff (prio 0, ram): next.ram
    000000000b000000-000000000b1cb0ff (prio 0, ram): next-video
00000000820c0000-00000000820c003f (prio 0, ram): alias next.bmapmem2 @next.bmapmem 0000000000000000-000000000000003f

In this case next.en is a dummy memory region which can easily be replaced with a proper device implementation: see the final version of next-cube.c after the series at https://gitlab.com/mcayland/qemu/-/blob/next-cube-improvements/hw/m68k/next-cube.c.


ATB,

Mark.


Reply via email to