Re: [Qemu-devel] [PATCH 1/8] rc4030: create custom DMA address space

[Paolo Bonzini]

On 05/03/2015 23:13, Hervé Poussineau wrote:
> +static const MemoryRegionOps rc4030_dma_tt_ops = {
> +    .impl.min_access_size = 4,
> +    .impl.max_access_size = 4,
> +    .impl.max_access_size = 4,
> +};
> +
> +static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
> +                                 uint32_t new_tl_limit)
> +{
> +    int entries, i;
> +    dma_pagetable_entry *dma_tl_contents;
> +
> +    if (s->dma_tl_limit) {
> +        /* write old dma tl table to physical memory */
> +        memory_region_del_subregion(get_system_memory(), &s->dma_tt);
> +        cpu_physical_memory_write(s->dma_tl_limit & 0x7fffffff,
> +                                  memory_region_get_ram_ptr(&s->dma_tt),
> +                                  s->dma_tl_limit);

You would need object_unparent(&s->dma_tt) here.

However, this breaks the rules for memory region lifetime (see
docs/memory.txt).

One solution is to warn here for a large dma_tl_limit and always create
a 4K ROMD region.  Here you can create an alias into the actual dma_tt
region and add/remove/unparent that alias.  Aliases can be created and
unparented at will.

> @@ -733,7 +793,16 @@ static void rc4030_do_dma(void *opaque, int n, uint8_t 
> *buf, int len, int is_wri
>      dma_addr = s->dma_regs[n][DMA_REG_ADDRESS];
>  
>      /* Read/write data at right place */
> -    rc4030_dma_memory_rw(opaque, dma_addr, buf, len, is_write);
> +    for (i = 0; i < len; ) {
> +        int ncpy = DMA_PAGESIZE - (dma_addr & (DMA_PAGESIZE - 1));
> +        if (ncpy > len - i) {
> +            ncpy = len - i;
> +        }
> +        address_space_rw(&s->dma_as, dma_addr, buf + i, ncpy, is_write);
> +
> +        dma_addr += ncpy;
> +        i += ncpy;
> +    }
>  
>      s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
>      s->dma_regs[n][DMA_REG_COUNT] -= len;

The loop should not be necessary, address_space_rw does the same.

Paolo

> @@ -800,6 +869,7 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>                    MemoryRegion *sysmem)
>  {
>      rc4030State *s;
> +    int i;
>  
>      s = g_malloc0(sizeof(rc4030State));
>  
> @@ -821,5 +891,15 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>                            "rc4030.jazzio", 0x00001000);
>      memory_region_add_subregion(sysmem, 0xf0000000, &s->iomem_jazzio);
>  
> +    memory_region_init(&s->dma_tt, NULL, "dma_tt", 0);
> +    memory_region_init(&s->dma_mr, NULL, "dma", INT32_MAX);
> +    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
> +        memory_region_init_alias(&s->dma_mrs[i], NULL, "dma-alias",
> +                                 get_system_memory(), 0, DMA_PAGESIZE);
> +        memory_region_set_enabled(&s->dma_mrs[i], false);
> +        memory_region_add_subregion(&s->dma_mr, i * DMA_PAGESIZE,
> +                                    &s->dma_mrs[i]);
> +    }
> +    address_space_init(&s->dma_as, &s->dma_mr, "rc4030_dma");
>      return s;
>  }
>