[Qemu-devel] Re: [PATCH 1/4] Add a scatter-gather list type and accessors

[Anthony Liguori <aliguori@us.ibm.com>]

Avi Kivity wrote:
> Scatter-gather lists are used extensively in dma-capable devices; a
> single data structure allows more code reuse later on.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  Makefile.target |    2 +-
>  dma-helpers.c   |   29 +++++++++++++++++++++++++++++
>  dma.h           |   24 ++++++++++++++++++++++++
>  3 files changed, 54 insertions(+), 1 deletions(-)
>  create mode 100644 dma-helpers.c
>  create mode 100644 dma.h
>
> diff --git a/Makefile.target b/Makefile.target
> index 372d185..28ba17f 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -500,7 +500,7 @@ endif #CONFIG_BSD_USER
>  # System emulator target
>  ifndef CONFIG_USER_ONLY
>  
> -OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o
> +OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o dma-helpers.o
>  # virtio has to be here due to weird dependency between PCI and virtio-net.
>  # need to fix this properly
>  OBJS+=virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
> diff --git a/dma-helpers.c b/dma-helpers.c
> new file mode 100644
> index 0000000..315834e
> --- /dev/null
> +++ b/dma-helpers.c
> @@ -0,0 +1,29 @@
>   

Needs copyright/license.

> +#include "dma.h"
> +
> +
> +void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint)
> +{
> +    qsg->sg = qemu_malloc(alloc_hint * sizeof(ScatterGatherEntry));
>   

Would be nice to check for malloc failures and fail gracefully at least.

> +    qsg->nsg = 0;
> +    qsg->nalloc = alloc_hint;
> +    qsg->size = 0;
> +}
> +
> +void qemu_sglist_add(QEMUSGList *qsg, target_phys_addr_t base,
> +                     target_phys_addr_t len)
> +{
> +    if (qsg->nsg == qsg->nalloc) {
> +        qsg->nalloc = 2 * qsg->nalloc + 1;
>   

Do you really want exponential growth verses linear growth?  The sg 
lists should be relatively small so linear growth should be fine.

Regards,

Anthony Liguori

> +        qsg->sg = qemu_realloc(qsg->sg, qsg->nalloc * sizeof(ScatterGatherEntry));
> +    }
> +    qsg->sg[qsg->nsg].base = base;
> +    qsg->sg[qsg->nsg].len = len;
> +    qsg->size += len;
> +    ++qsg->nsg;
> +}
> +
> +void qemu_sglist_destroy(QEMUSGList *qsg)
> +{
> +    qemu_free(qsg->sg);
> +}
> +
> diff --git a/dma.h b/dma.h
> new file mode 100644
> index 0000000..3b56fa6
> --- /dev/null
> +++ b/dma.h
> @@ -0,0 +1,24 @@
> +#ifndef DMA_H
> +#define DMA_H
> +
> +#include <stdio.h>
> +#include "cpu.h"
> +
> +typedef struct {
> +    target_phys_addr_t base;
> +    target_phys_addr_t len;
> +} ScatterGatherEntry;
> +
> +typedef struct {
> +    ScatterGatherEntry *sg;
> +    int nsg;
> +    int nalloc;
> +    target_phys_addr_t size;
> +} QEMUSGList;
> +
> +void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint);
> +void qemu_sglist_add(QEMUSGList *qsg, target_phys_addr_t base,
> +                     target_phys_addr_t len);
> +void qemu_sglist_destroy(QEMUSGList *qsg);
> +
> +#endif
>