* [Qemu-devel] [PATCH] Memory API conversion for mpic (openpic.c)
@ 2011-08-29 16:19 Fabien Chouteau
2011-08-30 12:20 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Fabien Chouteau @ 2011-08-29 16:19 UTC (permalink / raw)
To: qemu-devel; +Cc: avi
This patch converts mpic to the new memory API.
Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
hw/openpic.c | 208 +++++++++++++++++++++++-------------------------
hw/openpic.h | 8 +-
hw/ppce500_mpc8544ds.c | 3 +-
3 files changed, 105 insertions(+), 114 deletions(-)
diff --git a/hw/openpic.c b/hw/openpic.c
index 26c96e2..72be30c 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -116,18 +116,25 @@ enum {
#define MPIC_GLB_REG_START 0x0
#define MPIC_GLB_REG_SIZE 0x10F0
+#define MPIC_GLB_REG_END (MPIC_GLB_REG_START + MPIC_GLB_REG_SIZE - 1)
#define MPIC_TMR_REG_START 0x10F0
#define MPIC_TMR_REG_SIZE 0x220
+#define MPIC_TMR_REG_END (MPIC_TMR_REG_START + MPIC_TMR_REG_SIZE - 1)
#define MPIC_EXT_REG_START 0x10000
#define MPIC_EXT_REG_SIZE 0x180
+#define MPIC_EXT_REG_END (MPIC_EXT_REG_START + MPIC_EXT_REG_SIZE - 1)
#define MPIC_INT_REG_START 0x10200
#define MPIC_INT_REG_SIZE 0x800
+#define MPIC_INT_REG_END (MPIC_INT_REG_START + MPIC_INT_REG_SIZE - 1)
#define MPIC_MSG_REG_START 0x11600
#define MPIC_MSG_REG_SIZE 0x100
+#define MPIC_MSG_REG_END (MPIC_MSG_REG_START + MPIC_MSG_REG_SIZE - 1)
#define MPIC_MSI_REG_START 0x11C00
#define MPIC_MSI_REG_SIZE 0x100
+#define MPIC_MSI_REG_END (MPIC_MSI_REG_START + MPIC_MSI_REG_SIZE - 1)
#define MPIC_CPU_REG_START 0x20000
#define MPIC_CPU_REG_SIZE 0x100
+#define MPIC_CPU_REG_END (MPIC_CPU_REG_START + MPIC_CPU_REG_SIZE - 1)
enum mpic_ide_bits {
IDR_EP = 0,
@@ -1148,8 +1155,8 @@ static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src)
qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
}
-qemu_irq *openpic_init (PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
- qemu_irq **irqs, qemu_irq irq_out)
+qemu_irq *openpic_init(PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
+ qemu_irq **irqs, qemu_irq irq_out)
{
openpic_t *opp;
uint8_t *pci_conf;
@@ -1537,108 +1544,103 @@ static uint32_t mpic_src_msi_read (void *opaque, target_phys_addr_t addr)
return retval;
}
-static CPUWriteMemoryFunc * const mpic_glb_write[] = {
- &openpic_buggy_write,
- &openpic_buggy_write,
- &openpic_gbl_write,
-};
-
-static CPUReadMemoryFunc * const mpic_glb_read[] = {
- &openpic_buggy_read,
- &openpic_buggy_read,
- &openpic_gbl_read,
-};
-
-static CPUWriteMemoryFunc * const mpic_tmr_write[] = {
- &openpic_buggy_write,
- &openpic_buggy_write,
- &mpic_timer_write,
-};
-
-static CPUReadMemoryFunc * const mpic_tmr_read[] = {
- &openpic_buggy_read,
- &openpic_buggy_read,
- &mpic_timer_read,
-};
-
-static CPUWriteMemoryFunc * const mpic_cpu_write[] = {
- &openpic_buggy_write,
- &openpic_buggy_write,
- &openpic_cpu_write,
-};
-
-static CPUReadMemoryFunc * const mpic_cpu_read[] = {
- &openpic_buggy_read,
- &openpic_buggy_read,
- &openpic_cpu_read,
-};
-
-static CPUWriteMemoryFunc * const mpic_ext_write[] = {
- &openpic_buggy_write,
- &openpic_buggy_write,
- &mpic_src_ext_write,
-};
-
-static CPUReadMemoryFunc * const mpic_ext_read[] = {
- &openpic_buggy_read,
- &openpic_buggy_read,
- &mpic_src_ext_read,
-};
-
-static CPUWriteMemoryFunc * const mpic_int_write[] = {
- &openpic_buggy_write,
- &openpic_buggy_write,
- &mpic_src_int_write,
-};
+static uint64_t mpic_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
+{
+ uint64_t retval = (uint64_t)-1;
-static CPUReadMemoryFunc * const mpic_int_read[] = {
- &openpic_buggy_read,
- &openpic_buggy_read,
- &mpic_src_int_read,
-};
+ switch (size) {
+ case 4:
+ addr &= 0x3FFFF;
+ DPRINTF("%s: offset %08x\n", __func__, (int)addr);
+ switch (addr) {
+ case MPIC_GLB_REG_START ... MPIC_GLB_REG_END:
+ retval = openpic_gbl_read(opaque, addr & 0xffff);
+ break;
+ case MPIC_TMR_REG_START ... MPIC_TMR_REG_END:
+ retval = mpic_timer_read(opaque, addr & 0xfff);
+ break;
+ case MPIC_EXT_REG_START ... MPIC_EXT_REG_END:
+ retval = mpic_src_ext_read(opaque, addr & 0xfff);
+ break;
+ case MPIC_INT_REG_START ... MPIC_INT_REG_END:
+ retval = mpic_src_int_read(opaque, addr & 0xfff);
+ break;
+ case MPIC_MSG_REG_START ... MPIC_MSG_REG_END:
+ retval = mpic_src_msg_read(opaque, addr & 0xfff);
+ break;
+ case MPIC_MSI_REG_START ... MPIC_MSI_REG_END:
+ retval = mpic_src_msi_read(opaque, addr & 0xfff);
+ break;
+ case MPIC_CPU_REG_START ... MPIC_CPU_REG_END:
+ retval = openpic_cpu_read(opaque, addr & 0xfff);
+ break;
+ default:
+ DPRINTF("Invalid OPENPIC read access !\n");
+ }
+ break;
+ default:
+ DPRINTF("Invalid OPENPIC read access size:%d (must be 4)!\n", size);
+ }
+ return retval;
+}
-static CPUWriteMemoryFunc * const mpic_msg_write[] = {
- &openpic_buggy_write,
- &openpic_buggy_write,
- &mpic_src_msg_write,
-};
+static void mpic_write(void *opaque, target_phys_addr_t addr,
+ uint64_t data, unsigned size)
+{
+ switch (size) {
+ case 4:
+ addr &= 0x3FFFF;
+ DPRINTF("%s: offset %08x val: %08x\n", __func__,
+ (int)addr, (unsigned int)data);
+ switch (addr) {
+ case MPIC_GLB_REG_START ... MPIC_GLB_REG_END:
+ openpic_gbl_write(opaque, addr & 0xffff, data);
+ break;
+ case MPIC_TMR_REG_START ... MPIC_TMR_REG_END:
+ mpic_timer_write(opaque, addr & 0xfff, data);
+ break;
+ case MPIC_EXT_REG_START ... MPIC_EXT_REG_END:
+ mpic_src_ext_write(opaque, addr & 0xfff, data);
+ break;
+ case MPIC_INT_REG_START ... MPIC_INT_REG_END:
+ mpic_src_int_write(opaque, addr & 0xfff, data);
+ break;
+ case MPIC_MSG_REG_START ... MPIC_MSG_REG_END:
+ mpic_src_msg_write(opaque, addr & 0xfff, data);
+ break;
+ case MPIC_MSI_REG_START ... MPIC_MSI_REG_END:
+ mpic_src_msi_write(opaque, addr & 0xfff, data);
+ break;
+ case MPIC_CPU_REG_START ... MPIC_CPU_REG_END:
+ openpic_cpu_write(opaque, addr & 0xfff, data);
+ break;
+ default:
+ DPRINTF("Invalid OPENPIC write access !\n");
+ }
+ break;
-static CPUReadMemoryFunc * const mpic_msg_read[] = {
- &openpic_buggy_read,
- &openpic_buggy_read,
- &mpic_src_msg_read,
-};
-static CPUWriteMemoryFunc * const mpic_msi_write[] = {
- &openpic_buggy_write,
- &openpic_buggy_write,
- &mpic_src_msi_write,
-};
+ default:
+ DPRINTF("Invalid OPENPIC write access size:%d (must be 4)!\n", size);
+ return;
+ }
+}
-static CPUReadMemoryFunc * const mpic_msi_read[] = {
- &openpic_buggy_read,
- &openpic_buggy_read,
- &mpic_src_msi_read,
+static const MemoryRegionOps mpic_ops = {
+ .read = mpic_read,
+ .write = mpic_write,
+ .endianness = DEVICE_BIG_ENDIAN,
+ .impl = {
+ .min_access_size = 4,
+ .max_access_size = 4,
+ },
};
-qemu_irq *mpic_init (target_phys_addr_t base, int nb_cpus,
- qemu_irq **irqs, qemu_irq irq_out)
+qemu_irq *mpic_init(MemoryRegion *address_space, target_phys_addr_t base,
+ int nb_cpus, qemu_irq **irqs, qemu_irq irq_out)
{
openpic_t *mpp;
int i;
- struct {
- CPUReadMemoryFunc * const *read;
- CPUWriteMemoryFunc * const *write;
- target_phys_addr_t start_addr;
- ram_addr_t size;
- } const list[] = {
- {mpic_glb_read, mpic_glb_write, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
- {mpic_tmr_read, mpic_tmr_write, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
- {mpic_ext_read, mpic_ext_write, MPIC_EXT_REG_START, MPIC_EXT_REG_SIZE},
- {mpic_int_read, mpic_int_write, MPIC_INT_REG_START, MPIC_INT_REG_SIZE},
- {mpic_msg_read, mpic_msg_write, MPIC_MSG_REG_START, MPIC_MSG_REG_SIZE},
- {mpic_msi_read, mpic_msi_write, MPIC_MSI_REG_START, MPIC_MSI_REG_SIZE},
- {mpic_cpu_read, mpic_cpu_write, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
- };
/* XXX: for now, only one CPU is supported */
if (nb_cpus != 1)
@@ -1646,17 +1648,9 @@ qemu_irq *mpic_init (target_phys_addr_t base, int nb_cpus,
mpp = g_malloc0(sizeof(openpic_t));
- for (i = 0; i < sizeof(list)/sizeof(list[0]); i++) {
- int mem_index;
+ memory_region_init_io(&mpp->mem, &mpic_ops, mpp, "mpic", 0x40000);
- mem_index = cpu_register_io_memory(list[i].read, list[i].write, mpp,
- DEVICE_BIG_ENDIAN);
- if (mem_index < 0) {
- goto free;
- }
- cpu_register_physical_memory(base + list[i].start_addr,
- list[i].size, mem_index);
- }
+ memory_region_add_subregion(address_space, base, &mpp->mem);
mpp->nb_cpus = nb_cpus;
mpp->max_irq = MPIC_MAX_IRQ;
@@ -1674,8 +1668,4 @@ qemu_irq *mpic_init (target_phys_addr_t base, int nb_cpus,
qemu_register_reset(mpic_reset, mpp);
return qemu_allocate_irqs(openpic_set_irq, mpp, mpp->max_irq);
-
-free:
- g_free(mpp);
- return NULL;
}
diff --git a/hw/openpic.h b/hw/openpic.h
index 75de361..a9352c8 100644
--- a/hw/openpic.h
+++ b/hw/openpic.h
@@ -11,8 +11,8 @@ enum {
OPENPIC_OUTPUT_NB,
};
-qemu_irq *openpic_init (PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
- qemu_irq **irqs, qemu_irq irq_out);
-qemu_irq *mpic_init (target_phys_addr_t base, int nb_cpus,
- qemu_irq **irqs, qemu_irq irq_out);
+qemu_irq *openpic_init(PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
+ qemu_irq **irqs, qemu_irq irq_out);
+qemu_irq *mpic_init(MemoryRegion *address_space, target_phys_addr_t base,
+ int nb_cpus, qemu_irq **irqs, qemu_irq irq_out);
#endif /* __OPENPIC_H__ */
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 274b37c..7884627 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -272,7 +272,8 @@ static void mpc8544ds_init(ram_addr_t ram_size,
irqs = g_malloc0(sizeof(qemu_irq) * OPENPIC_OUTPUT_NB);
irqs[OPENPIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPCE500_INPUT_INT];
irqs[OPENPIC_OUTPUT_CINT] = ((qemu_irq *)env->irq_inputs)[PPCE500_INPUT_CINT];
- mpic = mpic_init(MPC8544_MPIC_REGS_BASE, 1, &irqs, NULL);
+ mpic = mpic_init(get_system_memory(), MPC8544_MPIC_REGS_BASE,
+ 1, &irqs, NULL);
/* Serial */
if (serial_hds[0]) {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] Memory API conversion for mpic (openpic.c)
2011-08-29 16:19 [Qemu-devel] [PATCH] Memory API conversion for mpic (openpic.c) Fabien Chouteau
@ 2011-08-30 12:20 ` Avi Kivity
2011-08-30 13:46 ` Fabien Chouteau
0 siblings, 1 reply; 4+ messages in thread
From: Avi Kivity @ 2011-08-30 12:20 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-devel
On 08/29/2011 07:19 PM, Fabien Chouteau wrote:
> This patch converts mpic to the new memory API.
>
> -static CPUReadMemoryFunc * const mpic_int_read[] = {
> -&openpic_buggy_read,
> -&openpic_buggy_read,
> -&mpic_src_int_read,
> -};
> + switch (size) {
> + case 4:
> + default:
> + DPRINTF("Invalid OPENPIC read access size:%d (must be 4)!\n", size);
Here, you accept multiple sizes.
> + }
> + return retval;
> +}
>
> -static CPUReadMemoryFunc * const mpic_msi_read[] = {
> -&openpic_buggy_read,
> -&openpic_buggy_read,
> -&mpic_src_msi_read,
> +static const MemoryRegionOps mpic_ops = {
> + .read = mpic_read,
> + .write = mpic_write,
> + .endianness = DEVICE_BIG_ENDIAN,
> + .impl = {
> + .min_access_size = 4,
> + .max_access_size = 4,
> + },
> };
Here, you reject them. One of the two is redundant.
>
> -qemu_irq *mpic_init (target_phys_addr_t base, int nb_cpus,
> - qemu_irq **irqs, qemu_irq irq_out)
> +qemu_irq *mpic_init(MemoryRegion *address_space, target_phys_addr_t base,
> + int nb_cpus, qemu_irq **irqs, qemu_irq irq_out)
> {
> openpic_t *mpp;
> int i;
> - struct {
> - CPUReadMemoryFunc * const *read;
> - CPUWriteMemoryFunc * const *write;
> - target_phys_addr_t start_addr;
> - ram_addr_t size;
> - } const list[] = {
> - {mpic_glb_read, mpic_glb_write, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
> - {mpic_tmr_read, mpic_tmr_write, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
> - {mpic_ext_read, mpic_ext_write, MPIC_EXT_REG_START, MPIC_EXT_REG_SIZE},
> - {mpic_int_read, mpic_int_write, MPIC_INT_REG_START, MPIC_INT_REG_SIZE},
> - {mpic_msg_read, mpic_msg_write, MPIC_MSG_REG_START, MPIC_MSG_REG_SIZE},
> - {mpic_msi_read, mpic_msi_write, MPIC_MSI_REG_START, MPIC_MSI_REG_SIZE},
> - {mpic_cpu_read, mpic_cpu_write, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
> - };
Why aren't you doing a 1:1 conversion? (i.e. generate a MemoryRegion
for every cpu_register_io_memory). I prefer those as being easier to
review. However if you have the means to test and have high confidence
in the patch's correctness, this way is acceptable.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] Memory API conversion for mpic (openpic.c)
2011-08-30 12:20 ` Avi Kivity
@ 2011-08-30 13:46 ` Fabien Chouteau
2011-08-30 13:48 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Fabien Chouteau @ 2011-08-30 13:46 UTC (permalink / raw)
To: Avi Kivity; +Cc: qemu-devel
On 30/08/2011 14:20, Avi Kivity wrote:
> On 08/29/2011 07:19 PM, Fabien Chouteau wrote:
>> This patch converts mpic to the new memory API.
>>
>> -static CPUReadMemoryFunc * const mpic_int_read[] = {
>> -&openpic_buggy_read,
>> -&openpic_buggy_read,
>> -&mpic_src_int_read,
>> -};
>> + switch (size) {
>> + case 4:
>
>
>> + default:
>> + DPRINTF("Invalid OPENPIC read access size:%d (must be 4)!\n", size);
>
> Here, you accept multiple sizes.
>
>> + }
>> + return retval;
>> +}
>>
>> -static CPUReadMemoryFunc * const mpic_msi_read[] = {
>> -&openpic_buggy_read,
>> -&openpic_buggy_read,
>> -&mpic_src_msi_read,
>> +static const MemoryRegionOps mpic_ops = {
>> + .read = mpic_read,
>> + .write = mpic_write,
>> + .endianness = DEVICE_BIG_ENDIAN,
>> + .impl = {
>> + .min_access_size = 4,
>> + .max_access_size = 4,
>> + },
>> };
>
> Here, you reject them. One of the two is redundant.
>
Right, I'll remove the second part and keep size handling in openpic.c as in
the current implementation.
>
>>
>> -qemu_irq *mpic_init (target_phys_addr_t base, int nb_cpus,
>> - qemu_irq **irqs, qemu_irq irq_out)
>> +qemu_irq *mpic_init(MemoryRegion *address_space, target_phys_addr_t base,
>> + int nb_cpus, qemu_irq **irqs, qemu_irq irq_out)
>> {
>> openpic_t *mpp;
>> int i;
>> - struct {
>> - CPUReadMemoryFunc * const *read;
>> - CPUWriteMemoryFunc * const *write;
>> - target_phys_addr_t start_addr;
>> - ram_addr_t size;
>> - } const list[] = {
>> - {mpic_glb_read, mpic_glb_write, MPIC_GLB_REG_START, MPIC_GLB_REG_SIZE},
>> - {mpic_tmr_read, mpic_tmr_write, MPIC_TMR_REG_START, MPIC_TMR_REG_SIZE},
>> - {mpic_ext_read, mpic_ext_write, MPIC_EXT_REG_START, MPIC_EXT_REG_SIZE},
>> - {mpic_int_read, mpic_int_write, MPIC_INT_REG_START, MPIC_INT_REG_SIZE},
>> - {mpic_msg_read, mpic_msg_write, MPIC_MSG_REG_START, MPIC_MSG_REG_SIZE},
>> - {mpic_msi_read, mpic_msi_write, MPIC_MSI_REG_START, MPIC_MSI_REG_SIZE},
>> - {mpic_cpu_read, mpic_cpu_write, MPIC_CPU_REG_START, MPIC_CPU_REG_SIZE},
>> - };
>
> Why aren't you doing a 1:1 conversion? (i.e. generate a MemoryRegion for
>every cpu_register_io_memory). I prefer those as being easier to review.
And more efficient than my dispatching, I guess.
Is it OK to use MemoryRegionOps.old_mmio in this case or should we avoid this deprecated
interface?
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] Memory API conversion for mpic (openpic.c)
2011-08-30 13:46 ` Fabien Chouteau
@ 2011-08-30 13:48 ` Avi Kivity
0 siblings, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2011-08-30 13:48 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: qemu-devel
On 08/30/2011 04:46 PM, Fabien Chouteau wrote:
> Is it OK to use MemoryRegionOps.old_mmio in this case or should we avoid this deprecated
> interface?
>
old_mmio is fine, esp. for the initial conversion.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-08-30 13:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-29 16:19 [Qemu-devel] [PATCH] Memory API conversion for mpic (openpic.c) Fabien Chouteau
2011-08-30 12:20 ` Avi Kivity
2011-08-30 13:46 ` Fabien Chouteau
2011-08-30 13:48 ` Avi Kivity
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.