Re: [Qemu-devel] SH: Improve the interrupt controller

[takasi-y]

# a patch attached is not to be commit to repos, but for explanation.
Hi,

> I have tested this only with SH4A and it's desirable to test
> with 7751/R2D. However, I no longer sure I know which kernel
> to use for that. Can anybody either provide me with instructions,
> or test this patch with R2D for me?

It doesn't work at least here for me (for r2d).
It stops with CPU load 100% (and I had to power my PC off...).

BTW, I think your patch is not following to current sh_intc's design model.
# please read to the end. this will be turned over later.

I think its design model is
 - registering memory regions for each registers.
 - read/write function doesn't check address
But, you have add address check in read/write function.

Mine(which I have forgotten to send..., only to move icr) is attached at the
 end of this mail. This one is based on the recognition above.

It works, but I wonder if this model is valid or not on mmio system after #5849.
Especially here is in question,
+    cpu_register_physical_memory(P4ADDR(base), 2, io_memory);
+    cpu_register_physical_memory(A7ADDR(base), 2, io_memory);

Paul(the author of #5849) said,
> [1] It's actually the offset from the start of the first page of that region. 
> In practice this difference doesn't matter, and makes the implementation a 
> lot simpler.

I don't know why he thought it "doesn't matter", but from discussions on ML,
 I guess the model he have is
 - register one memory region for each modules.
 - read/write function check address
 - most of modules are aligned to page.

In that case, your patch is nearer to the model.
But, then, we should done with these,
        cpu_register_physical_memory_offset(P4ADDR(address), 4,
                                            desc->iomemtype, INTC_A7(address));
        cpu_register_physical_memory_offset(A7ADDR(address), 4,
                                            desc->iomemtype, INTC_A7(address));

Sorry for no conclusion. I'm still wondering.
/yoshii

commit 53bf0820c255aea216053e67c11ffd409b78c16d
Author: Takashi YOSHII <address@hidden>
Date:   Thu Dec 11 22:26:19 2008 +0900

    move intc.icr from sh7750.c to sh_intc.c

diff --git a/hw/sh7750.c b/hw/sh7750.c
index 4d1a806..0c49c4c 100644
--- a/hw/sh7750.c
+++ b/hw/sh7750.c
@@ -700,7 +700,7 @@ SH7750State *sh7750_init(CPUSH4State * cpu)
     cpu_register_physical_memory(0xf0000000, 0x08000000,
                                 sh7750_mm_cache_and_tlb);
 
-    sh_intc_init(&s->intc, NR_SOURCES,
+    sh_intc_init(0x1fd00000, &s->intc, NR_SOURCES,
                 _INTC_ARRAY(mask_registers),
                 _INTC_ARRAY(prio_registers));
 
diff --git a/hw/sh_intc.c b/hw/sh_intc.c
index 136e7dd..7c66545 100644
--- a/hw/sh_intc.c
+++ b/hw/sh_intc.c
@@ -416,7 +416,26 @@ void sh_intc_register_sources(struct intc_desc *desc,
     }
 }
 
-int sh_intc_init(struct intc_desc *desc,
+static uint32_t sh_intc_icr_read(void *opaque, uint32_t offs)
+{
+    return ((struct intc_desc*)opaque)->icr;
+}
+
+static void sh_intc_icr_write(void *opaque, uint32_t offs, uint32_t value)
+{
+    ((struct intc_desc*)opaque)->icr = value;
+}
+
+static CPUReadMemoryFunc *sh_intc_icr_readfn[] = {
+    NULL, sh_intc_icr_read, NULL
+};
+
+static CPUWriteMemoryFunc *sh_intc_icr_writefn[] = {
+    NULL, sh_intc_icr_write, NULL
+};
+
+int sh_intc_init(target_phys_addr_t base,
+                struct intc_desc *desc,
                 int nr_sources,
                 struct intc_mask_reg *mask_regs,
                 int nr_mask_regs,
@@ -424,6 +443,7 @@ int sh_intc_init(struct intc_desc *desc,
                 int nr_prio_regs)
 {
     unsigned int i;
+    int io_memory;
 
     desc->pending = 0;
     desc->nr_sources = nr_sources;
@@ -465,6 +485,10 @@ int sh_intc_init(struct intc_desc *desc,
            sh_intc_register(desc, pr->clr_reg);
        }
     }
+    io_memory = cpu_register_io_memory(0, sh_intc_icr_readfn,
+                                       sh_intc_icr_writefn, desc);
+    cpu_register_physical_memory(P4ADDR(base), 2, io_memory);
+    cpu_register_physical_memory(A7ADDR(base), 2, io_memory);
 
     return 0;
 }
diff --git a/hw/sh_intc.h b/hw/sh_intc.h
index 4e36f00..e39f794 100644
--- a/hw/sh_intc.h
+++ b/hw/sh_intc.h
@@ -55,6 +55,7 @@ struct intc_desc {
     int nr_prio_regs;
     int iomemtype;
     int pending; /* number of interrupt sources that has pending set */
+    uint16_t icr;
 };
 
 int sh_intc_get_pending_vector(struct intc_desc *desc, int imask);
@@ -68,7 +69,8 @@ void sh_intc_register_sources(struct intc_desc *desc,
                              struct intc_group *groups,
                              int nr_groups);
 
-int sh_intc_init(struct intc_desc *desc,
+int sh_intc_init(target_phys_addr_t base,
+                struct intc_desc *desc,
                 int nr_sources,
                 struct intc_mask_reg *mask_regs,
                 int nr_mask_regs,