From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sasha Levin Subject: [RFC 12/12] kvm tools: Fixes for ioeventfd module Date: Mon, 19 Dec 2011 15:58:34 +0200 Message-ID: <1324303114-5948-13-git-send-email-levinsasha928@gmail.com> References: <1324303114-5948-1-git-send-email-levinsasha928@gmail.com> Cc: mingo@elte.hu, gorcunov@gmail.com, asias.hejun@gmail.com, kvm@vger.kernel.org, Sasha Levin To: penberg@kernel.org Return-path: Received: from mail-ww0-f44.google.com ([74.125.82.44]:37768 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753104Ab1LSN70 (ORCPT ); Mon, 19 Dec 2011 08:59:26 -0500 Received: by mail-ww0-f44.google.com with SMTP id dr13so10761138wgb.1 for ; Mon, 19 Dec 2011 05:59:25 -0800 (PST) In-Reply-To: <1324303114-5948-1-git-send-email-levinsasha928@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: Fixes include: - Error handling - Cleanup - Standard init/uninit Signed-off-by: Sasha Levin --- tools/kvm/builtin-run.c | 8 ++- tools/kvm/include/kvm/kvm.h | 6 +- tools/kvm/include/kvm/virtio-pci.h | 1 + tools/kvm/include/kvm/virtio-rng.h | 4 +- tools/kvm/include/kvm/virtio-trans.h | 1 + tools/kvm/ioport.c | 9 ++- tools/kvm/kvm.c | 101 +++++++++++++++++++++++----------- tools/kvm/mmio.c | 8 ++- tools/kvm/util/rbtree-interval.c | 9 ++- tools/kvm/virtio/pci.c | 34 ++++++++++- tools/kvm/virtio/rng.c | 27 +++++++-- tools/kvm/virtio/trans.c | 4 +- tools/kvm/x86/irq.c | 17 +++++- 13 files changed, 168 insertions(+), 61 deletions(-) diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c index 5b198e0..defd1b9 100644 --- a/tools/kvm/builtin-run.c +++ b/tools/kvm/builtin-run.c @@ -1238,7 +1238,9 @@ static void kvm_cmd_run_uninit(int guest_ret) if (r < 0) pr_warning("virtio_blk__uninit() failed with error %d\n", r); - virtio_rng__delete_all(kvm); + r = virtio_rng__uninit(kvm); + if (r < 0) + pr_warning("virtio_rng__uninit() failed with error %d\n", r); r = disk_image__close_all(kvm->disks, image_count); if (r < 0) @@ -1268,7 +1270,9 @@ static void kvm_cmd_run_uninit(int guest_ret) if (r < 0) pr_warning("pci__uninit() failed with error %d\n", r); - kvm__delete(kvm); + r = kvm__uninit(kvm); + if (r < 0) + pr_warning("pci__uninit() failed with error %d\n", r); if (guest_ret == 0) printf("\n # KVM session ended normally.\n"); diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h index 74f46c1..e7603a9 100644 --- a/tools/kvm/include/kvm/kvm.h +++ b/tools/kvm/include/kvm/kvm.h @@ -32,7 +32,7 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s int kvm__recommended_cpus(struct kvm *kvm); int kvm__max_cpus(struct kvm *kvm); void kvm__init_ram(struct kvm *kvm); -void kvm__delete(struct kvm *kvm); +int kvm__uninit(struct kvm *kvm); bool kvm__load_kernel(struct kvm *kvm, const char *kernel_filename, const char *initrd_filename, const char *kernel_cmdline, u16 vidmode); void kvm__start_timer(struct kvm *kvm); @@ -41,8 +41,8 @@ void kvm__irq_line(struct kvm *kvm, int irq, int level); void kvm__irq_trigger(struct kvm *kvm, int irq); bool kvm__emulate_io(struct kvm *kvm, u16 port, void *data, int direction, int size, u32 count); bool kvm__emulate_mmio(struct kvm *kvm, u64 phys_addr, u8 *data, u32 len, u8 is_write); -void kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr); -bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, +int kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr); +int kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, void (*mmio_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), void *ptr); bool kvm__deregister_mmio(struct kvm *kvm, u64 phys_addr); diff --git a/tools/kvm/include/kvm/virtio-pci.h b/tools/kvm/include/kvm/virtio-pci.h index 73f7486..a2d43e6 100644 --- a/tools/kvm/include/kvm/virtio-pci.h +++ b/tools/kvm/include/kvm/virtio-pci.h @@ -40,6 +40,7 @@ struct virtio_pci { int virtio_pci__init(struct kvm *kvm, struct virtio_trans *vtrans, void *dev, int device_id, int subsys_id, int class); +int virtio_pci__uninit(struct kvm *kvm, struct virtio_trans *vtrans); int virtio_pci__signal_vq(struct kvm *kvm, struct virtio_trans *vtrans, u32 vq); int virtio_pci__signal_config(struct kvm *kvm, struct virtio_trans *vtrans); diff --git a/tools/kvm/include/kvm/virtio-rng.h b/tools/kvm/include/kvm/virtio-rng.h index c0a413b..983dfbf 100644 --- a/tools/kvm/include/kvm/virtio-rng.h +++ b/tools/kvm/include/kvm/virtio-rng.h @@ -3,7 +3,7 @@ struct kvm; -void virtio_rng__init(struct kvm *kvm); -void virtio_rng__delete_all(struct kvm *kvm); +int virtio_rng__init(struct kvm *kvm); +int virtio_rng__uninit(struct kvm *kvm); #endif /* KVM__RNG_VIRTIO_H */ diff --git a/tools/kvm/include/kvm/virtio-trans.h b/tools/kvm/include/kvm/virtio-trans.h index e7c186e..f2e433f 100644 --- a/tools/kvm/include/kvm/virtio-trans.h +++ b/tools/kvm/include/kvm/virtio-trans.h @@ -27,6 +27,7 @@ struct virtio_ops { struct virtio_trans_ops { int (*init)(struct kvm *kvm, struct virtio_trans *vtrans, void *dev, int device_id, int subsys_id, int class); + int (*uninit)(struct kvm *kvm, struct virtio_trans *vtrans); int (*signal_vq)(struct kvm *kvm, struct virtio_trans *virtio_trans, u32 queueid); int (*signal_config)(struct kvm *kvm, struct virtio_trans *virtio_trans); }; diff --git a/tools/kvm/ioport.c b/tools/kvm/ioport.c index 19caf6c..5237ba5 100644 --- a/tools/kvm/ioport.c +++ b/tools/kvm/ioport.c @@ -60,6 +60,7 @@ static void ioport_remove(struct rb_root *root, struct ioport *data) int ioport__register(u16 port, struct ioport_operations *ops, int count, void *param) { struct ioport *entry; + int r; br_write_lock(); if (port == IOPORT_EMPTY) @@ -81,8 +82,12 @@ int ioport__register(u16 port, struct ioport_operations *ops, int count, void *p .priv = param, }; - ioport_insert(&ioport_tree, entry); - + r = ioport_insert(&ioport_tree, entry); + if (r < 0) { + free(entry); + br_write_unlock(); + return r; + } br_write_unlock(); return port; diff --git a/tools/kvm/kvm.c b/tools/kvm/kvm.c index 8c9e268..32d05b1 100644 --- a/tools/kvm/kvm.c +++ b/tools/kvm/kvm.c @@ -6,6 +6,7 @@ #include "kvm/kvm-ipc.h" #include +#include #include #include @@ -62,7 +63,7 @@ extern struct kvm_ext kvm_req_ext[]; static char kvm_dir[PATH_MAX]; -static void set_dir(const char *fmt, va_list args) +static int set_dir(const char *fmt, va_list args) { char tmp[PATH_MAX]; @@ -71,9 +72,11 @@ static void set_dir(const char *fmt, va_list args) mkdir(tmp, 0777); if (!realpath(tmp, kvm_dir)) - die("Unable to set KVM tool directory"); + return -errno; strcat(kvm_dir, "/"); + + return 0; } void kvm__set_dir(const char *fmt, ...) @@ -103,7 +106,7 @@ bool kvm__supports_extension(struct kvm *kvm, unsigned int extension) static int kvm__check_extensions(struct kvm *kvm) { - unsigned int i; + int i; for (i = 0; ; i++) { if (!kvm_req_ext[i].name) @@ -111,7 +114,7 @@ static int kvm__check_extensions(struct kvm *kvm) if (!kvm__supports_extension(kvm, kvm_req_ext[i].code)) { pr_error("Unsuppored KVM extension detected: %s", kvm_req_ext[i].name); - return (int)-i; + return -i; } } @@ -120,10 +123,10 @@ static int kvm__check_extensions(struct kvm *kvm) static struct kvm *kvm__new(void) { - struct kvm *kvm = calloc(1, sizeof *kvm); + struct kvm *kvm = calloc(1, sizeof(*kvm)); if (!kvm) - die("out of memory"); + return ERR_PTR(-ENOMEM); return kvm; } @@ -136,11 +139,13 @@ static int kvm__create_socket(struct kvm *kvm) int len, r; if (!kvm->name) - return -1; + return -EINVAL; sprintf(full_name, "%s/%s.sock", kvm__get_dir(), kvm->name); - if (access(full_name, F_OK) == 0) - die("Socket file %s already exist", full_name); + if (access(full_name, F_OK) == 0) { + pr_error("Socket file %s already exist", full_name); + return -EEXIST; + } s = socket(AF_UNIX, SOCK_STREAM, 0); if (s < 0) @@ -161,7 +166,7 @@ static int kvm__create_socket(struct kvm *kvm) fail: close(s); - return -1; + return r; } void kvm__remove_socket(const char *name) @@ -189,9 +194,9 @@ int kvm__get_sock_by_instance(const char *name) if (r < 0 && errno == ECONNREFUSED) { /* Clean ghost socket file */ unlink(sock_file); - return -1; + return r; } else if (r < 0) { - die("Failed connecting to instance"); + return r; } return s; @@ -206,7 +211,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd)) dir = opendir(kvm__get_dir()); if (!dir) - return -1; + return -errno; for (;;) { readdir_r(dir, &entry, &result); @@ -229,7 +234,7 @@ int kvm__enumerate_instances(int (*callback)(const char *name, int fd)) return ret; } -void kvm__delete(struct kvm *kvm) +int kvm__uninit(struct kvm *kvm) { kvm__stop_timer(kvm); @@ -237,6 +242,8 @@ void kvm__delete(struct kvm *kvm) kvm_ipc__stop(); kvm__remove_socket(kvm->name); free(kvm); + + return 0; } /* @@ -244,7 +251,7 @@ void kvm__delete(struct kvm *kvm) * memory regions to it. Therefore, be careful if you use this function for * registering memory regions for emulating hardware. */ -void kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr) +int kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspace_addr) { struct kvm_userspace_memory_region mem; int ret; @@ -258,7 +265,9 @@ void kvm__register_mem(struct kvm *kvm, u64 guest_phys, u64 size, void *userspac ret = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &mem); if (ret < 0) - die_perror("KVM_SET_USER_MEMORY_REGION ioctl"); + return -errno; + + return 0; } int kvm__recommended_cpus(struct kvm *kvm) @@ -312,33 +321,53 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s struct kvm *kvm; int ret; - if (!kvm__arch_cpu_supports_vm()) - die("Your CPU does not support hardware virtualization"); + if (!kvm__arch_cpu_supports_vm()) { + pr_error("Your CPU does not support hardware virtualization"); + return ERR_PTR(-ENOSYS); + } kvm = kvm__new(); + if (IS_ERR_OR_NULL(kvm)) + return kvm; kvm->sys_fd = open(kvm_dev, O_RDWR); if (kvm->sys_fd < 0) { - if (errno == ENOENT) - die("'%s' not found. Please make sure your kernel has CONFIG_KVM enabled and that the KVM modules are loaded.", kvm_dev); - if (errno == ENODEV) - die("'%s' KVM driver not available.\n # (If the KVM module is loaded then 'dmesg' may offer further clues about the failure.)", kvm_dev); - - fprintf(stderr, " Fatal, could not open %s: ", kvm_dev); - perror(NULL); - exit(1); + if (errno == ENOENT) { + pr_error("'%s' not found. Please make sure your kernel has CONFIG_KVM " + "enabled and that the KVM modules are loaded.", kvm_dev); + ret = -errno; + goto cleanup; + } + if (errno == ENODEV) { + die("'%s' KVM driver not available.\n # (If the KVM " + "module is loaded then 'dmesg' may offer further clues " + "about the failure.)", kvm_dev); + ret = -errno; + goto cleanup; + } + + pr_error("Could not open %s: ", kvm_dev); + ret = -errno; + goto cleanup; } ret = ioctl(kvm->sys_fd, KVM_GET_API_VERSION, 0); - if (ret != KVM_API_VERSION) - die_perror("KVM_API_VERSION ioctl"); + if (ret != KVM_API_VERSION) { + pr_error("KVM_API_VERSION ioctl"); + ret = -errno; + goto cleanup; + } kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0); - if (kvm->vm_fd < 0) - die_perror("KVM_CREATE_VM ioctl"); - - if (kvm__check_extensions(kvm)) - die("A required KVM extention is not supported by OS"); + if (kvm->vm_fd < 0) { + ret = kvm->vm_fd; + goto cleanup; + } + + if (kvm__check_extensions(kvm)) { + pr_error("A required KVM extention is not supported by OS"); + ret = -ENOSYS; + } kvm__arch_init(kvm, kvm_dev, hugetlbfs_path, ram_size, name); @@ -347,6 +376,12 @@ struct kvm *kvm__init(const char *kvm_dev, const char *hugetlbfs_path, u64 ram_s kvm_ipc__start(kvm__create_socket(kvm)); kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid); return kvm; +cleanup: + close(kvm->vm_fd); + close(kvm->sys_fd); + free(kvm); + + return ERR_PTR(ret); } /* RFC 1952 */ diff --git a/tools/kvm/mmio.c b/tools/kvm/mmio.c index 6e8d1bb..7760a7f 100644 --- a/tools/kvm/mmio.c +++ b/tools/kvm/mmio.c @@ -9,6 +9,8 @@ #include #include #include +#include +#include #define mmio_node(n) rb_entry(n, struct mmio_mapping, node) @@ -56,7 +58,7 @@ static const char *to_direction(u8 is_write) return "read"; } -bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, +int kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, void (*mmio_fn)(u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), void *ptr) { @@ -66,7 +68,7 @@ bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool mmio = malloc(sizeof(*mmio)); if (mmio == NULL) - return false; + return -ENOMEM; *mmio = (struct mmio_mapping) { .node = RB_INT_INIT(phys_addr, phys_addr + phys_addr_len), @@ -82,7 +84,7 @@ bool kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool ret = ioctl(kvm->vm_fd, KVM_REGISTER_COALESCED_MMIO, &zone); if (ret < 0) { free(mmio); - return false; + return -errno; } } br_write_lock(); diff --git a/tools/kvm/util/rbtree-interval.c b/tools/kvm/util/rbtree-interval.c index edc140d..f9bf4b8 100644 --- a/tools/kvm/util/rbtree-interval.c +++ b/tools/kvm/util/rbtree-interval.c @@ -1,5 +1,6 @@ #include #include +#include struct rb_int_node *rb_int_search_single(struct rb_root *root, u64 point) { @@ -56,10 +57,10 @@ static void update_node_max_high(struct rb_node *node, void *arg) int rb_int_insert(struct rb_root *root, struct rb_int_node *i_node) { - struct rb_node **node = &(root->rb_node), *parent = NULL; + struct rb_node **node = &(root->rb_node), *parent = NULL; while (*node) { - int result = i_node->low - rb_int(*node)->low; + int result = i_node->low - rb_int(*node)->low; parent = *node; if (result < 0) @@ -67,14 +68,14 @@ int rb_int_insert(struct rb_root *root, struct rb_int_node *i_node) else if (result > 0) node = &((*node)->rb_right); else - return 0; + return -EEXIST; } rb_link_node(&i_node->node, parent, node); rb_insert_color(&i_node->node, root); rb_augment_insert(&i_node->node, update_node_max_high, NULL); - return 1; + return 0; } void rb_int_erase(struct rb_root *root, struct rb_int_node *node) diff --git a/tools/kvm/virtio/pci.c b/tools/kvm/virtio/pci.c index 7cc014e..fb46d5c 100644 --- a/tools/kvm/virtio/pci.c +++ b/tools/kvm/virtio/pci.c @@ -18,6 +18,7 @@ struct virtio_trans_ops *virtio_pci__get_trans_ops(void) .signal_vq = virtio_pci__signal_vq, .signal_config = virtio_pci__signal_config, .init = virtio_pci__init, + .uninit = virtio_pci__uninit, }; return &virtio_pci_trans; }; @@ -297,7 +298,9 @@ int virtio_pci__init(struct kvm *kvm, struct virtio_trans *vtrans, void *dev, return r; vpci->base_addr = (u16)r; - kvm__register_mmio(kvm, vpci->msix_io_block, PCI_IO_SIZE, false, callback_mmio_table, vpci); + r = kvm__register_mmio(kvm, vpci->msix_io_block, PCI_IO_SIZE, false, callback_mmio_table, vpci); + if (r < 0) + goto free_ioport; vpci->pci_hdr = (struct pci_device_header) { .vendor_id = cpu_to_le16(PCI_VENDOR_ID_REDHAT_QUMRANET), @@ -343,12 +346,35 @@ int virtio_pci__init(struct kvm *kvm, struct virtio_trans *vtrans, void *dev, vpci->pci_hdr.msix.pba_offset = cpu_to_le32(1 | PCI_IO_SIZE); /* Use BAR 3 */ vpci->config_vector = 0; - if (irq__register_device(subsys_id, &ndev, &pin, &line) < 0) - return -1; + r = irq__register_device(subsys_id, &ndev, &pin, &line); + if (r < 0) + goto free_mmio; vpci->pci_hdr.irq_pin = pin; vpci->pci_hdr.irq_line = line; - pci__register(&vpci->pci_hdr, ndev); + r = pci__register(&vpci->pci_hdr, ndev); + if (r < 0) + goto free_ioport; return 0; + +free_mmio: + kvm__deregister_mmio(kvm, vpci->msix_io_block); +free_ioport: + ioport__unregister(vpci->base_addr); + return r; } + +int virtio_pci__uninit(struct kvm *kvm, struct virtio_trans *vtrans) +{ + struct virtio_pci *vpci = vtrans->virtio; + int i; + + kvm__deregister_mmio(kvm, vpci->msix_io_block); + ioport__unregister(vpci->base_addr); + + for (i = 0; i < VIRTIO_PCI_MAX_VQ; i++) + ioeventfd__del_event(vpci->base_addr + VIRTIO_PCI_QUEUE_NOTIFY, i); + + return 0; +} \ No newline at end of file diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c index c9430cb..cbbfbeb 100644 --- a/tools/kvm/virtio/rng.c +++ b/tools/kvm/virtio/rng.c @@ -149,21 +149,27 @@ static struct virtio_ops rng_dev_virtio_ops = (struct virtio_ops) { .get_size_vq = get_size_vq, }; -void virtio_rng__init(struct kvm *kvm) +int virtio_rng__init(struct kvm *kvm) { struct rng_dev *rdev; + int r; rdev = malloc(sizeof(*rdev)); if (rdev == NULL) - return; + return -ENOMEM; rdev->fd = open("/dev/urandom", O_RDONLY); - if (rdev->fd < 0) - die("Failed initializing RNG"); + if (rdev->fd < 0) { + r = rdev->fd; + goto cleanup; + } virtio_trans_init(&rdev->vtrans, VIRTIO_PCI); - rdev->vtrans.trans_ops->init(kvm, &rdev->vtrans, rdev, PCI_DEVICE_ID_VIRTIO_RNG, + r = rdev->vtrans.trans_ops->init(kvm, &rdev->vtrans, rdev, PCI_DEVICE_ID_VIRTIO_RNG, VIRTIO_ID_RNG, PCI_CLASS_RNG); + if (r < 0) + goto cleanup; + rdev->vtrans.virtio_ops = &rng_dev_virtio_ops; list_add_tail(&rdev->list, &rdevs); @@ -175,14 +181,23 @@ void virtio_rng__init(struct kvm *kvm) "Please make sure that the guest kernel was " "compiled with CONFIG_HW_RANDOM_VIRTIO=y enabled " "in its .config"); + return 0; +cleanup: + close(rdev->fd); + free(rdev); + + return r; } -void virtio_rng__delete_all(struct kvm *kvm) +int virtio_rng__uninit(struct kvm *kvm) { struct rng_dev *rdev, *tmp; list_for_each_entry_safe(rdev, tmp, &rdevs, list) { list_del(&rdev->list); + rdev->vtrans.trans_ops->uninit(kvm, &rdev->vtrans); free(rdev); } + + return 0; } diff --git a/tools/kvm/virtio/trans.c b/tools/kvm/virtio/trans.c index 50c206d..cd4fc7e 100644 --- a/tools/kvm/virtio/trans.c +++ b/tools/kvm/virtio/trans.c @@ -13,10 +13,12 @@ int virtio_trans_init(struct virtio_trans *vtrans, enum virtio_trans_type type) case VIRTIO_PCI: trans = calloc(sizeof(struct virtio_pci), 1); if (!trans) - die("Failed allocating virtio transport"); + return -ENOMEM; vtrans->virtio = trans; vtrans->trans_ops = virtio_pci__get_trans_ops(); default: return -1; }; + + return 0; } \ No newline at end of file diff --git a/tools/kvm/x86/irq.c b/tools/kvm/x86/irq.c index 3683cb4..4ee5e56 100644 --- a/tools/kvm/x86/irq.c +++ b/tools/kvm/x86/irq.c @@ -46,7 +46,7 @@ static struct pci_dev *search(struct rb_root *root, u32 id) struct rb_node *node = root->rb_node; while (node) { - struct pci_dev *data = container_of(node, struct pci_dev, node); + struct pci_dev *data = rb_entry(node, struct pci_dev, node); int result; result = id - data->id; @@ -175,8 +175,23 @@ int irq__init(struct kvm *kvm) int irq__uninit(struct kvm *kvm) { + struct rb_node *ent; + free(irq_routing); + for (ent = rb_first(&pci_tree); ent; ent = rb_next(ent)) { + struct pci_dev *dev; + struct irq_line *line; + struct list_head *node, *tmp; + + dev = rb_entry(ent, struct pci_dev, node); + list_for_each_safe(node, tmp, &dev->lines) { + line = list_entry(node, struct irq_line, node); + free(line); + } + free(dev); + } + return 0; } -- 1.7.8