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
