[1/2] Minimal RAM API support

This adds a minimum chunk of Anthony's RAM API support so that we
can identify actual VM RAM versus all the other things that make
use of qemu_ram_alloc.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 Makefile.target |    1 +
 cpu-common.h    |    2 +
 memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.h        |   23 +++++++++++++++
 4 files changed, 108 insertions(+), 0 deletions(-)
 create mode 100644 memory.c
 create mode 100644 memory.h

On Fri, Oct 29, 2010 at 4:39 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> This adds a minimum chunk of Anthony's RAM API support so that we
> can identify actual VM RAM versus all the other things that make
> use of qemu_ram_alloc.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
>  Makefile.target |    1 +
>  cpu-common.h    |    2 +
>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory.h        |   23 +++++++++++++++
>  4 files changed, 108 insertions(+), 0 deletions(-)
>  create mode 100644 memory.c
>  create mode 100644 memory.h
>
> diff --git a/Makefile.target b/Makefile.target
> index c48cbcc..e4e2eb4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>  obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> +obj-y += memory.o

Please move this to Makefile.objs to compile the object in hwlib.
There are no target dependencies.

>  LIBS+=-lz
>
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> diff --git a/cpu-common.h b/cpu-common.h
> index a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>  /* address in the RAM (different from a physical address) */
>  typedef unsigned long ram_addr_t;
>
> +#include "memory.h"
> +
>  /* memory API */
>
>  typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
> diff --git a/memory.c b/memory.c
> new file mode 100644
> index 0000000..86947fb
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,82 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };

Please avoid global state. This is not used elsewhere, so it could be
static. But instead the API should take a state parameter
(RAMSlotState *) so that no static state is needed.

> +
> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr && slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;
> +}
> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..91e552e
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,23 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    void *host;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;

This definition should be in memory.c.

On Fri, 2010-10-29 at 19:57 +0000, Blue Swirl wrote:
> On Fri, Oct 29, 2010 at 4:39 PM, Alex Williamson
> <alex.williamson@redhat.com> wrote:
> > This adds a minimum chunk of Anthony's RAM API support so that we
> > can identify actual VM RAM versus all the other things that make
> > use of qemu_ram_alloc.
> >
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >
> >  Makefile.target |    1 +
> >  cpu-common.h    |    2 +
> >  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  memory.h        |   23 +++++++++++++++
> >  4 files changed, 108 insertions(+), 0 deletions(-)
> >  create mode 100644 memory.c
> >  create mode 100644 memory.h
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index c48cbcc..e4e2eb4 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
> >  obj-y += rwhandler.o
> >  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
> >  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> > +obj-y += memory.o
> 
> Please move this to Makefile.objs to compile the object in hwlib.
> There are no target dependencies.

Ok, will do.

> >  LIBS+=-lz
> >
> >  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> > diff --git a/cpu-common.h b/cpu-common.h
> > index a543b5d..6aa2738 100644
> > --- a/cpu-common.h
> > +++ b/cpu-common.h
> > @@ -23,6 +23,8 @@
> >  /* address in the RAM (different from a physical address) */
> >  typedef unsigned long ram_addr_t;
> >
> > +#include "memory.h"
> > +
> >  /* memory API */
> >
> >  typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
> > diff --git a/memory.c b/memory.c
> > new file mode 100644
> > index 0000000..86947fb
> > --- /dev/null
> > +++ b/memory.c
> > @@ -0,0 +1,82 @@
> > +/*
> > + * RAM API
> > + *
> > + *  Copyright Red Hat, Inc. 2010
> > + *
> > + * Authors:
> > + *  Alex Williamson <alex.williamson@redhat.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#include "memory.h"
> > +#include "range.h"
> > +
> > +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
> 
> Please avoid global state. This is not used elsewhere, so it could be
> static. But instead the API should take a state parameter
> (RAMSlotState *) so that no static state is needed.

The reason for this not being static is that the vfio driver I'm working
on walks it.  Also the reason for the definition being in memory.h
instead of memory.c as you've noted below.  Probably better to solve
that usage by creating an interface that calls a function pointer for
each entry... I'll work on that.  Thanks,

Alex

> > +
> > +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> > +                                       ram_addr_t size)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> > +        if (slot->start_addr == start_addr && slot->size == size) {
> > +            return slot;
> > +        }
> > +
> > +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> > +            abort();
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> > +                       ram_addr_t phys_offset)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    if (!size) {
> > +        return;
> > +    }
> > +
> > +    assert(!qemu_ram_find_slot(start_addr, size));
> > +
> > +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> > +
> > +    slot->start_addr = start_addr;
> > +    slot->size = size;
> > +    slot->offset = phys_offset;
> > +
> > +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> > +
> > +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> > +}
> > +
> > +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> > +{
> > +    QemuRamSlot *slot;
> > +
> > +    if (!size) {
> > +        return;
> > +    }
> > +
> > +    slot = qemu_ram_find_slot(start_addr, size);
> > +    assert(slot != NULL);
> > +
> > +    QLIST_REMOVE(slot, next);
> > +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> > +
> > +    return;
> > +}
> > diff --git a/memory.h b/memory.h
> > new file mode 100644
> > index 0000000..91e552e
> > --- /dev/null
> > +++ b/memory.h
> > @@ -0,0 +1,23 @@
> > +#ifndef QEMU_MEMORY_H
> > +#define QEMU_MEMORY_H
> > +
> > +#include "qemu-common.h"
> > +#include "cpu-common.h"
> > +
> > +typedef struct QemuRamSlot {
> > +    target_phys_addr_t start_addr;
> > +    ram_addr_t size;
> > +    ram_addr_t offset;
> > +    void *host;
> > +    QLIST_ENTRY(QemuRamSlot) next;
> > +} QemuRamSlot;
> > +
> > +typedef struct QemuRamSlots {
> > +    QLIST_HEAD(slots, QemuRamSlot) slots;
> > +} QemuRamSlots;
> 
> This definition should be in memory.c.

On Fri, Oct 29, 2010 at 10:39:03AM -0600, Alex Williamson wrote:
> This adds a minimum chunk of Anthony's RAM API support so that we
> can identify actual VM RAM versus all the other things that make
> use of qemu_ram_alloc.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
>  Makefile.target |    1 +
>  cpu-common.h    |    2 +
>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  memory.h        |   23 +++++++++++++++
>  4 files changed, 108 insertions(+), 0 deletions(-)
>  create mode 100644 memory.c
>  create mode 100644 memory.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index c48cbcc..e4e2eb4 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>  obj-y += rwhandler.o
>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
> +obj-y += memory.o
>  LIBS+=-lz
>  
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> diff --git a/cpu-common.h b/cpu-common.h
> index a543b5d..6aa2738 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -23,6 +23,8 @@
>  /* address in the RAM (different from a physical address) */
>  typedef unsigned long ram_addr_t;
>  
> +#include "memory.h"
> +
>  /* memory API */
>  
>  typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
> diff --git a/memory.c b/memory.c
> new file mode 100644
> index 0000000..86947fb
> --- /dev/null
> +++ b/memory.c
> @@ -0,0 +1,82 @@
> +/*
> + * RAM API
> + *
> + *  Copyright Red Hat, Inc. 2010
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "memory.h"
> +#include "range.h"
> +
> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
> +
> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
> +                                       ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
> +        if (slot->start_addr == start_addr && slot->size == size) {
> +            return slot;
> +        }
> +
> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
> +            abort();
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    assert(!qemu_ram_find_slot(start_addr, size));
> +
> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
> +
> +    slot->start_addr = start_addr;
> +    slot->size = size;
> +    slot->offset = phys_offset;
> +
> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
> +
> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
> +}
> +
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
> +{
> +    QemuRamSlot *slot;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    slot = qemu_ram_find_slot(start_addr, size);
> +    assert(slot != NULL);
> +
> +    QLIST_REMOVE(slot, next);
> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
> +
> +    return;
> +}

qemu_free(slot) is necessary.


> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 0000000..91e552e
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,23 @@
> +#ifndef QEMU_MEMORY_H
> +#define QEMU_MEMORY_H
> +
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +
> +typedef struct QemuRamSlot {
> +    target_phys_addr_t start_addr;
> +    ram_addr_t size;
> +    ram_addr_t offset;
> +    void *host;
> +    QLIST_ENTRY(QemuRamSlot) next;
> +} QemuRamSlot;
> +
> +typedef struct QemuRamSlots {
> +    QLIST_HEAD(slots, QemuRamSlot) slots;
> +} QemuRamSlots;
> +extern QemuRamSlots ram_slots;
> +
> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
> +                       ram_addr_t phys_offset);
> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size);
> +#endif
> 
>

On Sun, Oct 31, 2010 at 8:17 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Fri, Oct 29, 2010 at 10:39:03AM -0600, Alex Williamson wrote:
>> This adds a minimum chunk of Anthony's RAM API support so that we
>> can identify actual VM RAM versus all the other things that make
>> use of qemu_ram_alloc.
>>
>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>> ---
>>
>>  Makefile.target |    1 +
>>  cpu-common.h    |    2 +
>>  memory.c        |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  memory.h        |   23 +++++++++++++++
>>  4 files changed, 108 insertions(+), 0 deletions(-)
>>  create mode 100644 memory.c
>>  create mode 100644 memory.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index c48cbcc..e4e2eb4 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -175,6 +175,7 @@ obj-$(CONFIG_VIRTFS) += virtio-9p.o
>>  obj-y += rwhandler.o
>>  obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>>  obj-$(CONFIG_NO_KVM) += kvm-stub.o
>> +obj-y += memory.o
>>  LIBS+=-lz
>>
>>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>> diff --git a/cpu-common.h b/cpu-common.h
>> index a543b5d..6aa2738 100644
>> --- a/cpu-common.h
>> +++ b/cpu-common.h
>> @@ -23,6 +23,8 @@
>>  /* address in the RAM (different from a physical address) */
>>  typedef unsigned long ram_addr_t;
>>
>> +#include "memory.h"
>> +
>>  /* memory API */
>>
>>  typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, uint32_t value);
>> diff --git a/memory.c b/memory.c
>> new file mode 100644
>> index 0000000..86947fb
>> --- /dev/null
>> +++ b/memory.c
>> @@ -0,0 +1,82 @@
>> +/*
>> + * RAM API
>> + *
>> + *  Copyright Red Hat, Inc. 2010
>> + *
>> + * Authors:
>> + *  Alex Williamson <alex.williamson@redhat.com>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include "memory.h"
>> +#include "range.h"
>> +
>> +QemuRamSlots ram_slots = { .slots = QLIST_HEAD_INITIALIZER(ram_slots) };
>> +
>> +static QemuRamSlot *qemu_ram_find_slot(target_phys_addr_t start_addr,
>> +                                       ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    QLIST_FOREACH(slot, &ram_slots.slots, next) {
>> +        if (slot->start_addr == start_addr && slot->size == size) {
>> +            return slot;
>> +        }
>> +
>> +        if (ranges_overlap(start_addr, size, slot->start_addr, slot->size)) {
>> +            abort();
>> +        }
>> +    }
>> +
>> +    return NULL;
>> +}
>> +
>> +void qemu_ram_register(target_phys_addr_t start_addr, ram_addr_t size,
>> +                       ram_addr_t phys_offset)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    if (!size) {
>> +        return;
>> +    }
>> +
>> +    assert(!qemu_ram_find_slot(start_addr, size));
>> +
>> +    slot = qemu_mallocz(sizeof(QemuRamSlot));
>> +
>> +    slot->start_addr = start_addr;
>> +    slot->size = size;
>> +    slot->offset = phys_offset;
>> +
>> +    QLIST_INSERT_HEAD(&ram_slots.slots, slot, next);
>> +
>> +    cpu_register_physical_memory(slot->start_addr, slot->size, slot->offset);
>> +}
>> +
>> +void qemu_ram_unregister(target_phys_addr_t start_addr, ram_addr_t size)
>> +{
>> +    QemuRamSlot *slot;
>> +
>> +    if (!size) {
>> +        return;
>> +    }
>> +
>> +    slot = qemu_ram_find_slot(start_addr, size);
>> +    assert(slot != NULL);
>> +
>> +    QLIST_REMOVE(slot, next);
>> +    cpu_register_physical_memory(start_addr, size, IO_MEM_UNASSIGNED);
>> +
>> +    return;
>> +}
>
> qemu_free(slot) is necessary.

Thank you!

Alex