From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QyOeW-0000Xh-WA for qemu-devel@nongnu.org; Tue, 30 Aug 2011 09:46:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QyOeV-0008Vr-SE for qemu-devel@nongnu.org; Tue, 30 Aug 2011 09:46:56 -0400 Received: from mel.act-europe.fr ([194.98.77.210]:55392) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QyOeV-0008Vl-Hj for qemu-devel@nongnu.org; Tue, 30 Aug 2011 09:46:55 -0400 Message-ID: <4E5CE9BF.1070300@adacore.com> Date: Tue, 30 Aug 2011 15:46:39 +0200 From: Fabien Chouteau MIME-Version: 1.0 References: <1314634742-14219-1-git-send-email-chouteau@adacore.com> <4E5CD594.40903@redhat.com> In-Reply-To: <4E5CD594.40903@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Memory API conversion for mpic (openpic.c) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: qemu-devel@nongnu.org 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