[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 35/38] ivshmem: Clean up after the previous comm
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH 35/38] ivshmem: Clean up after the previous commit |
Date: |
Thu, 3 Mar 2016 14:56:51 +0100 |
On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <address@hidden> wrote:
> Move code to more sensible places. Use the opportunity to reorder and
> document IVShmemState members.
>
> Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
> ---
> hw/misc/ivshmem.c | 420
> +++++++++++++++++++++++++++---------------------------
> 1 file changed, 213 insertions(+), 207 deletions(-)
>
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index f7f5b3b..33b6842 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -85,25 +85,30 @@ typedef struct IVShmemState {
> PCIDevice parent_obj;
> /*< public >*/
>
> - HostMemoryBackend *hostmem;
> + uint32_t features;
> +
> + /* exactly one of these two may be set */
> + HostMemoryBackend *hostmem; /* with interrupts */
> + CharDriverState *server_chr; /* without interrupts */
> +
> + /* registers */
> uint32_t intrmask;
> uint32_t intrstatus;
> + int vm_id;
>
> - CharDriverState *server_chr;
> - MemoryRegion ivshmem_mmio;
> -
> + /* BARs */
> + MemoryRegion ivshmem_mmio; /* BAR 0 (registers) */
> MemoryRegion *ivshmem_bar2; /* BAR 2 (shared memory) */
>
> + /* interrupt support */
> Peer *peers;
> int nb_peers; /* space in @peers[] */
> -
> - int vm_id;
> uint32_t vectors;
> - uint32_t features;
> MSIVector *msi_vectors;
> uint64_t msg_buf; /* buffer for receiving server messages */
> int msg_buffered_bytes; /* #bytes in @msg_buf */
>
> + /* migration stuff */
> OnOffAuto master;
> Error *migration_blocker;
>
> @@ -812,33 +817,6 @@ static void ivshmem_write_config(PCIDevice *pdev,
> uint32_t address,
> }
> }
>
> -static HostMemoryBackend *desugar_shm(const char *shm, size_t size)
> -{
> - /* TODO avoid the detour through QemuOpts */
> - static int counter;
> - QemuOpts *opts = qemu_opts_create(qemu_find_opts("object"),
> - NULL, 0, &error_abort);
> - char *path;
> - Object *obj;
> -
> - qemu_opt_set(opts, "qom-type", "memory-backend-file",
> - &error_abort);
> - /* FIXME need a better way to make up an ID */
> - qemu_opts_set_id(opts, g_strdup_printf("ivshmem-backend-%d", counter++));
> - path = g_strdup_printf("/dev/shm/%s", shm);
> - qemu_opt_set(opts, "mem-path", path, &error_abort);
> - qemu_opt_set_number(opts, "size", size, &error_abort);
> - qemu_opt_set_bool(opts, "share", true, &error_abort);
> - g_free(path);
> -
> - obj = user_creatable_add_opts(opts, &error_abort);
> - qemu_opts_del(opts);
> -
> - user_creatable_complete(obj, &error_abort);
> -
> - return MEMORY_BACKEND(obj);
> -}
> -
> static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
> {
> IVShmemState *s = IVSHMEM_COMMON(dev);
> @@ -914,65 +892,6 @@ static void ivshmem_common_realize(PCIDevice *dev, Error
> **errp)
> }
> }
>
> -static void ivshmem_realize(PCIDevice *dev, Error **errp)
> -{
> - IVShmemState *s = IVSHMEM_COMMON(dev);
> -
> - if (!qtest_enabled()) {
> - error_report("ivshmem is deprecated, please use ivshmem-plain"
> - " or ivshmem-doorbell instead");
> - }
> -
> - if (!!s->server_chr + !!s->shmobj + !!s->hostmem != 1) {
> - error_setg(errp,
> - "You must specify either 'shm', 'chardev' or 'x-memdev'");
> - return;
> - }
> -
> - if (s->hostmem) {
> - if (s->sizearg) {
> - g_warning("size argument ignored with hostmem");
> - }
> - } else if (s->sizearg == NULL) {
> - s->legacy_size = 4 << 20; /* 4 MB default */
> - } else {
> - char *end;
> - int64_t size = qemu_strtosz(s->sizearg, &end);
> - if (size < 0 || (size_t)size != size || *end != '\0'
> - || !is_power_of_2(size)) {
> - error_setg(errp, "Invalid size %s", s->sizearg);
> - return;
> - }
> - s->legacy_size = size;
> - }
> -
> - /* check that role is reasonable */
> - if (s->role) {
> - if (strncmp(s->role, "peer", 5) == 0) {
> - s->master = ON_OFF_AUTO_OFF;
> - } else if (strncmp(s->role, "master", 7) == 0) {
> - s->master = ON_OFF_AUTO_ON;
> - } else {
> - error_setg(errp, "'role' must be 'peer' or 'master'");
> - return;
> - }
> - } else {
> - s->master = ON_OFF_AUTO_AUTO;
> - }
> -
> - if (s->shmobj) {
> - s->hostmem = desugar_shm(s->shmobj, s->legacy_size);
> - }
> -
> - /*
> - * Note: we don't use INTx with IVSHMEM_MSI at all, so this is a
> - * bald-faced lie then. But it's a backwards compatible lie.
> - */
> - pci_config_set_interrupt_pin(dev->config, 1);
> -
> - ivshmem_common_realize(dev, errp);
> -}
> -
> static void ivshmem_exit(PCIDevice *dev)
> {
> IVShmemState *s = IVSHMEM_COMMON(dev);
> @@ -1012,18 +931,6 @@ static void ivshmem_exit(PCIDevice *dev)
> g_free(s->msi_vectors);
> }
>
> -static bool test_msix(void *opaque, int version_id)
> -{
> - IVShmemState *s = opaque;
> -
> - return ivshmem_has_feature(s, IVSHMEM_MSI);
> -}
> -
> -static bool test_no_msix(void *opaque, int version_id)
> -{
> - return !test_msix(opaque, version_id);
> -}
> -
> static int ivshmem_pre_load(void *opaque)
> {
> IVShmemState *s = opaque;
> @@ -1042,70 +949,6 @@ static int ivshmem_post_load(void *opaque, int
> version_id)
> return 0;
> }
>
> -static int ivshmem_load_old(QEMUFile *f, void *opaque, int version_id)
> -{
> - IVShmemState *s = opaque;
> - PCIDevice *pdev = PCI_DEVICE(s);
> - int ret;
> -
> - IVSHMEM_DPRINTF("ivshmem_load_old\n");
> -
> - if (version_id != 0) {
> - return -EINVAL;
> - }
> -
> - ret = ivshmem_pre_load(s);
> - if (ret) {
> - return ret;
> - }
> -
> - ret = pci_device_load(pdev, f);
> - if (ret) {
> - return ret;
> - }
> -
> - if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
> - msix_load(pdev, f);
> - } else {
> - s->intrstatus = qemu_get_be32(f);
> - s->intrmask = qemu_get_be32(f);
> - }
> - ivshmem_vector_use(s);
> -
> - return 0;
> -}
> -
> -static const VMStateDescription ivshmem_vmsd = {
> - .name = "ivshmem",
> - .version_id = 1,
> - .minimum_version_id = 1,
> - .pre_load = ivshmem_pre_load,
> - .post_load = ivshmem_post_load,
> - .fields = (VMStateField[]) {
> - VMSTATE_PCI_DEVICE(parent_obj, IVShmemState),
> -
> - VMSTATE_MSIX_TEST(parent_obj, IVShmemState, test_msix),
> - VMSTATE_UINT32_TEST(intrstatus, IVShmemState, test_no_msix),
> - VMSTATE_UINT32_TEST(intrmask, IVShmemState, test_no_msix),
> -
> - VMSTATE_END_OF_LIST()
> - },
> - .load_state_old = ivshmem_load_old,
> - .minimum_version_id_old = 0
> -};
> -
> -static Property ivshmem_properties[] = {
> - DEFINE_PROP_CHR("chardev", IVShmemState, server_chr),
> - DEFINE_PROP_STRING("size", IVShmemState, sizearg),
> - DEFINE_PROP_UINT32("vectors", IVShmemState, vectors, 1),
> - DEFINE_PROP_BIT("ioeventfd", IVShmemState, features, IVSHMEM_IOEVENTFD,
> false),
> - DEFINE_PROP_BIT("msi", IVShmemState, features, IVSHMEM_MSI, true),
> - DEFINE_PROP_STRING("shm", IVShmemState, shmobj),
> - DEFINE_PROP_STRING("role", IVShmemState, role),
> - DEFINE_PROP_UINT32("use64", IVShmemState, not_legacy_32bit, 1),
> - DEFINE_PROP_END_OF_LIST(),
> -};
> -
> static void ivshmem_common_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1123,17 +966,13 @@ static void ivshmem_common_class_init(ObjectClass
> *klass, void *data)
> dc->desc = "Inter-VM shared memory";
> }
>
> -static void ivshmem_class_init(ObjectClass *klass, void *data)
> -{
> - DeviceClass *dc = DEVICE_CLASS(klass);
> - PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> -
> - k->realize = ivshmem_realize;
> - k->revision = 0;
> - dc->desc = "Inter-VM shared memory (legacy)";
> - dc->props = ivshmem_properties;
> - dc->vmsd = &ivshmem_vmsd;
> -}
> +static const TypeInfo ivshmem_common_info = {
> + .name = TYPE_IVSHMEM_COMMON,
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(IVShmemState),
> + .abstract = true,
> + .class_init = ivshmem_common_class_init,
> +};
>
> static void ivshmem_check_memdev_is_busy(Object *obj, const char *name,
> Object *val, Error **errp)
> @@ -1150,33 +989,6 @@ static void ivshmem_check_memdev_is_busy(Object *obj,
> const char *name,
> }
> }
>
> -static void ivshmem_init(Object *obj)
> -{
> - IVShmemState *s = IVSHMEM(obj);
> -
> - object_property_add_link(obj, "x-memdev", TYPE_MEMORY_BACKEND,
> - (Object **)&s->hostmem,
> - ivshmem_check_memdev_is_busy,
> - OBJ_PROP_LINK_UNREF_ON_RELEASE,
> - &error_abort);
> -}
> -
> -static const TypeInfo ivshmem_common_info = {
> - .name = TYPE_IVSHMEM_COMMON,
> - .parent = TYPE_PCI_DEVICE,
> - .instance_size = sizeof(IVShmemState),
> - .abstract = true,
> - .class_init = ivshmem_common_class_init,
> -};
> -
> -static const TypeInfo ivshmem_info = {
> - .name = TYPE_IVSHMEM,
> - .parent = TYPE_IVSHMEM_COMMON,
> - .instance_size = sizeof(IVShmemState),
> - .instance_init = ivshmem_init,
> - .class_init = ivshmem_class_init,
> -};
> -
> static const VMStateDescription ivshmem_plain_vmsd = {
> .name = TYPE_IVSHMEM_PLAIN,
> .version_id = 0,
> @@ -1271,6 +1083,200 @@ static const TypeInfo ivshmem_doorbell_info = {
> .class_init = ivshmem_doorbell_class_init,
> };
>
> +static int ivshmem_load_old(QEMUFile *f, void *opaque, int version_id)
> +{
> + IVShmemState *s = opaque;
> + PCIDevice *pdev = PCI_DEVICE(s);
> + int ret;
> +
> + IVSHMEM_DPRINTF("ivshmem_load_old\n");
> +
> + if (version_id != 0) {
> + return -EINVAL;
> + }
> +
> + ret = ivshmem_pre_load(s);
> + if (ret) {
> + return ret;
> + }
> +
> + ret = pci_device_load(pdev, f);
> + if (ret) {
> + return ret;
> + }
> +
> + if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
> + msix_load(pdev, f);
> + } else {
> + s->intrstatus = qemu_get_be32(f);
> + s->intrmask = qemu_get_be32(f);
> + }
> + ivshmem_vector_use(s);
> +
> + return 0;
> +}
> +
> +static bool test_msix(void *opaque, int version_id)
> +{
> + IVShmemState *s = opaque;
> +
> + return ivshmem_has_feature(s, IVSHMEM_MSI);
> +}
> +
> +static bool test_no_msix(void *opaque, int version_id)
> +{
> + return !test_msix(opaque, version_id);
> +}
> +
> +static const VMStateDescription ivshmem_vmsd = {
> + .name = "ivshmem",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .pre_load = ivshmem_pre_load,
> + .post_load = ivshmem_post_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_PCI_DEVICE(parent_obj, IVShmemState),
> +
> + VMSTATE_MSIX_TEST(parent_obj, IVShmemState, test_msix),
> + VMSTATE_UINT32_TEST(intrstatus, IVShmemState, test_no_msix),
> + VMSTATE_UINT32_TEST(intrmask, IVShmemState, test_no_msix),
> +
> + VMSTATE_END_OF_LIST()
> + },
> + .load_state_old = ivshmem_load_old,
> + .minimum_version_id_old = 0
> +};
> +
> +static Property ivshmem_properties[] = {
> + DEFINE_PROP_CHR("chardev", IVShmemState, server_chr),
> + DEFINE_PROP_STRING("size", IVShmemState, sizearg),
> + DEFINE_PROP_UINT32("vectors", IVShmemState, vectors, 1),
> + DEFINE_PROP_BIT("ioeventfd", IVShmemState, features, IVSHMEM_IOEVENTFD,
> + false),
> + DEFINE_PROP_BIT("msi", IVShmemState, features, IVSHMEM_MSI, true),
> + DEFINE_PROP_STRING("shm", IVShmemState, shmobj),
> + DEFINE_PROP_STRING("role", IVShmemState, role),
> + DEFINE_PROP_UINT32("use64", IVShmemState, not_legacy_32bit, 1),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static HostMemoryBackend *desugar_shm(const char *shm, size_t size)
> +{
> + /* TODO avoid the detour through QemuOpts */
> + static int counter;
> + QemuOpts *opts = qemu_opts_create(qemu_find_opts("object"),
> + NULL, 0, &error_abort);
> + char *path;
> + Object *obj;
> +
> + qemu_opt_set(opts, "qom-type", "memory-backend-file",
> + &error_abort);
> + /* FIXME need a better way to make up an ID */
> + qemu_opts_set_id(opts, g_strdup_printf("ivshmem-backend-%d", counter++));
> + path = g_strdup_printf("/dev/shm/%s", shm);
> + qemu_opt_set(opts, "mem-path", path, &error_abort);
> + qemu_opt_set_number(opts, "size", size, &error_abort);
> + qemu_opt_set_bool(opts, "share", true, &error_abort);
> + g_free(path);
> +
> + obj = user_creatable_add_opts(opts, &error_abort);
> + qemu_opts_del(opts);
> +
> + user_creatable_complete(obj, &error_abort);
> +
> + return MEMORY_BACKEND(obj);
> +}
> +
> +static void ivshmem_realize(PCIDevice *dev, Error **errp)
> +{
> + IVShmemState *s = IVSHMEM_COMMON(dev);
> +
> + if (!qtest_enabled()) {
> + error_report("ivshmem is deprecated, please use ivshmem-plain"
> + " or ivshmem-doorbell instead");
> + }
> +
> + if (!!s->server_chr + !!s->shmobj + !!s->hostmem != 1) {
> + error_setg(errp,
> + "You must specify either 'shm', 'chardev' or 'x-memdev'");
> + return;
> + }
> +
> + if (s->hostmem) {
> + if (s->sizearg) {
> + g_warning("size argument ignored with hostmem");
> + }
> + } else if (s->sizearg == NULL) {
> + s->legacy_size = 4 << 20; /* 4 MB default */
> + } else {
> + char *end;
> + int64_t size = qemu_strtosz(s->sizearg, &end);
> + if (size < 0 || (size_t)size != size || *end != '\0'
> + || !is_power_of_2(size)) {
> + error_setg(errp, "Invalid size %s", s->sizearg);
> + return;
> + }
> + s->legacy_size = size;
> + }
> +
> + /* check that role is reasonable */
> + if (s->role) {
> + if (strncmp(s->role, "peer", 5) == 0) {
> + s->master = ON_OFF_AUTO_OFF;
> + } else if (strncmp(s->role, "master", 7) == 0) {
> + s->master = ON_OFF_AUTO_ON;
> + } else {
> + error_setg(errp, "'role' must be 'peer' or 'master'");
> + return;
> + }
> + } else {
> + s->master = ON_OFF_AUTO_AUTO;
> + }
> +
> + if (s->shmobj) {
> + s->hostmem = desugar_shm(s->shmobj, s->legacy_size);
> + }
> +
> + /*
> + * Note: we don't use INTx with IVSHMEM_MSI at all, so this is a
> + * bald-faced lie then. But it's a backwards compatible lie.
> + */
> + pci_config_set_interrupt_pin(dev->config, 1);
> +
> + ivshmem_common_realize(dev, errp);
> +}
> +
> +static void ivshmem_init(Object *obj)
> +{
> + IVShmemState *s = IVSHMEM(obj);
> +
> + object_property_add_link(obj, "x-memdev", TYPE_MEMORY_BACKEND,
> + (Object **)&s->hostmem,
> + ivshmem_check_memdev_is_busy,
> + OBJ_PROP_LINK_UNREF_ON_RELEASE,
> + &error_abort);
> +}
> +
> +static void ivshmem_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> + k->realize = ivshmem_realize;
> + k->revision = 0;
> + dc->desc = "Inter-VM shared memory (legacy)";
> + dc->props = ivshmem_properties;
> + dc->vmsd = &ivshmem_vmsd;
> +}
> +
> +static const TypeInfo ivshmem_info = {
> + .name = TYPE_IVSHMEM,
> + .parent = TYPE_IVSHMEM_COMMON,
> + .instance_size = sizeof(IVShmemState),
> + .instance_init = ivshmem_init,
> + .class_init = ivshmem_class_init,
> +};
> +
> static void ivshmem_register_types(void)
> {
> type_register_static(&ivshmem_common_info);
> --
> 2.4.3
>
>
--
Marc-André Lureau
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 35/38] ivshmem: Clean up after the previous commit,
Marc-André Lureau <=