* [PATCH 1/3] kvm tools: Introduce IRQ registry
@ 2011-05-06 11:24 Sasha Levin
2011-05-06 11:24 ` [PATCH 2/3] kvm tools: Dynamically add devices when creating mptable Sasha Levin
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Sasha Levin @ 2011-05-06 11:24 UTC (permalink / raw)
To: penberg; +Cc: mingo, asias.hejun, gorcunov, prasadjoshi124, kvm, Sasha Levin
Instead of having static definitions of devices, Use a
dynamic registry of pci devices.
The structure is a rbtree which holds device types (net,
blk, etc). Each device entry holds a list of IRQ lines
associated with that device (pin).
Devices dynamically register upon initialization, and receive
a set of: device id, irq pin and irq line.
Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
tools/kvm/Makefile | 2 +
tools/kvm/include/kvm/irq.h | 24 +++++++++
tools/kvm/include/linux/module.h | 6 ++
tools/kvm/irq.c | 107 ++++++++++++++++++++++++++++++++++++++
4 files changed, 139 insertions(+), 0 deletions(-)
create mode 100644 tools/kvm/include/kvm/irq.h
create mode 100644 tools/kvm/include/linux/module.h
create mode 100644 tools/kvm/irq.c
diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index 89c7e3d..fb839fc 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -39,6 +39,8 @@ OBJS += kvm-run.o
OBJS += qcow.o
OBJS += mptable.o
OBJS += threadpool.o
+OBJS += irq.o
+OBJS += ../../lib/rbtree.o
DEPS := $(patsubst %.o,%.d,$(OBJS))
diff --git a/tools/kvm/include/kvm/irq.h b/tools/kvm/include/kvm/irq.h
new file mode 100644
index 0000000..7a75a0c
--- /dev/null
+++ b/tools/kvm/include/kvm/irq.h
@@ -0,0 +1,24 @@
+#ifndef KVM__IRQ_H
+#define KVM__IRQ_H
+
+#include <linux/types.h>
+#include <linux/rbtree.h>
+#include <linux/list.h>
+
+struct irq_line {
+ u8 line;
+ struct list_head node;
+};
+
+struct pci_dev {
+ struct rb_node node;
+ u32 id;
+ u8 pin;
+ struct list_head lines;
+};
+
+int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line);
+
+struct rb_node *irq__get_pci_tree(void);
+
+#endif
diff --git a/tools/kvm/include/linux/module.h b/tools/kvm/include/linux/module.h
new file mode 100644
index 0000000..0e4c6a3
--- /dev/null
+++ b/tools/kvm/include/linux/module.h
@@ -0,0 +1,6 @@
+#ifndef KVM__LINUX_MODULE_H
+#define KVM__LINUX_MODULE_H
+
+#define EXPORT_SYMBOL(name)
+
+#endif
diff --git a/tools/kvm/irq.c b/tools/kvm/irq.c
new file mode 100644
index 0000000..4f4305f
--- /dev/null
+++ b/tools/kvm/irq.c
@@ -0,0 +1,107 @@
+#include "kvm/irq.h"
+
+#include <linux/types.h>
+#include <linux/rbtree.h>
+#include <linux/list.h>
+
+#include <stddef.h>
+#include <stdlib.h>
+
+static u8 next_pin = 1;
+static u8 next_line = 3;
+static u8 next_dev = 1;
+static struct rb_root pci_tree = RB_ROOT;
+
+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);
+ int result;
+
+ result = id - data->id;
+
+ if (result < 0)
+ node = node->rb_left;
+ else if (result > 0)
+ node = node->rb_right;
+ else
+ return data;
+ }
+ return NULL;
+}
+
+static int insert(struct rb_root *root, struct pci_dev *data)
+{
+ struct rb_node **new = &(root->rb_node), *parent = NULL;
+
+ /* Figure out where to put new node */
+ while (*new) {
+ struct pci_dev *this = container_of(*new, struct pci_dev, node);
+ int result = data->id - this->id;
+
+ parent = *new;
+ if (result < 0)
+ new = &((*new)->rb_left);
+ else if (result > 0)
+ new = &((*new)->rb_right);
+ else
+ return 0;
+ }
+
+ /* Add new node and rebalance tree. */
+ rb_link_node(&data->node, parent, new);
+ rb_insert_color(&data->node, root);
+
+ return 1;
+}
+
+int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line)
+{
+ struct pci_dev *node;
+
+ node = search(&pci_tree, dev);
+
+ if (!node) {
+ /* We haven't found a node - First device of it's kind */
+ node = malloc(sizeof(*node));
+ if (node == NULL)
+ return -1;
+
+ *node = (struct pci_dev) {
+ .id = dev,
+ .pin = next_pin++,
+ };
+
+ INIT_LIST_HEAD(&node->lines);
+
+ if (insert(&pci_tree, node) != 1) {
+ free(node);
+ return -1;
+ }
+ }
+
+ if (node) {
+ /* This device already has a pin assigned, give out a new line and device id */
+ struct irq_line *new = malloc(sizeof(*new));
+ if (new == NULL)
+ return -1;
+
+ new->line = next_line++;
+ *line = new->line;
+ *pin = node->pin;
+ *num = next_dev++;
+
+ list_add(&new->node, &node->lines);
+
+ return 0;
+ }
+
+ return -1;
+}
+
+struct rb_node *irq__get_pci_tree(void)
+{
+ return rb_first(&pci_tree);
+}
--
1.7.5.rc3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] kvm tools: Dynamically add devices when creating mptable 2011-05-06 11:24 [PATCH 1/3] kvm tools: Introduce IRQ registry Sasha Levin @ 2011-05-06 11:24 ` Sasha Levin 2011-05-06 11:24 ` [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry Sasha Levin 2011-05-07 9:13 ` [PATCH 1/3] kvm tools: Introduce " Pekka Enberg 2 siblings, 0 replies; 7+ messages in thread From: Sasha Levin @ 2011-05-06 11:24 UTC (permalink / raw) To: penberg; +Cc: mingo, asias.hejun, gorcunov, prasadjoshi124, kvm, Sasha Levin Enumerate registered devices to build a complete and updated mptable containing all registered pci devices. Signed-off-by: Sasha Levin <levinsasha928@gmail.com> --- tools/kvm/kvm-run.c | 4 ++-- tools/kvm/mptable.c | 30 +++++++++++------------------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c index d5a952f..bb228fe 100644 --- a/tools/kvm/kvm-run.c +++ b/tools/kvm/kvm-run.c @@ -446,8 +446,6 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) rtc__init(); - kvm__setup_bios(kvm); - serial8250__init(kvm); pci__init(); @@ -479,6 +477,8 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix) kvm__start_timer(kvm); + kvm__setup_bios(kvm); + for (i = 0; i < nrcpus; i++) { kvm_cpus[i] = kvm_cpu__init(kvm, i); if (!kvm_cpus[i]) diff --git a/tools/kvm/mptable.c b/tools/kvm/mptable.c index 5a24336..c75d9ce 100644 --- a/tools/kvm/mptable.c +++ b/tools/kvm/mptable.c @@ -3,6 +3,7 @@ #include "kvm/apic.h" #include "kvm/mptable.h" #include "kvm/util.h" +#include "kvm/irq.h" #include <linux/kernel.h> #include <string.h> @@ -87,6 +88,7 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus) struct mpc_bus *mpc_bus; struct mpc_ioapic *mpc_ioapic; struct mpc_intsrc *mpc_intsrc; + struct rb_node *pci_tree; const int pcibusid = 0; const int isabusid = 1; @@ -186,30 +188,20 @@ void mptable_setup(struct kvm *kvm, unsigned int ncpus) * * Also note we use PCI irqs here, no for ISA bus yet. */ - mpc_intsrc = last_addr; - /* src irq = virtio console irq pin, dst irq = virtio console irq */ - mptable_add_irq_src(mpc_intsrc, pcibusid, 2, ioapicid, 13); - last_addr = (void *)&mpc_intsrc[1]; - nentries++; + for (pci_tree = irq__get_pci_tree(); pci_tree; pci_tree = rb_next(pci_tree)) { + struct pci_dev *dev = rb_entry(pci_tree, struct pci_dev, node); + struct irq_line *tmp; - /* Currently we define 4 possible virtio-blk devices */ - for (i = 0; i < 4; i++) { - mpc_intsrc = last_addr; + list_for_each_entry(tmp, &dev->lines, node) { + mpc_intsrc = last_addr; - /* src irq = virtio blk irq pin, dst irq = virtio blk irq */ - mptable_add_irq_src(mpc_intsrc, pcibusid, 1, ioapicid, 9 + i); - last_addr = (void *)&mpc_intsrc[1]; - nentries++; + mptable_add_irq_src(mpc_intsrc, pcibusid, dev->pin, ioapicid, tmp->line); + last_addr = (void *)&mpc_intsrc[1]; + nentries++; + } } - mpc_intsrc = last_addr; - - /* src irq = virtio net irq pin, dst irq = virtio net irq */ - mptable_add_irq_src(mpc_intsrc, pcibusid, 3, ioapicid, 14); - last_addr = (void *)&mpc_intsrc[1]; - nentries++; - /* * Local IRQs assignment (LINT0, LINT1) */ -- 1.7.5.rc3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry 2011-05-06 11:24 [PATCH 1/3] kvm tools: Introduce IRQ registry Sasha Levin 2011-05-06 11:24 ` [PATCH 2/3] kvm tools: Dynamically add devices when creating mptable Sasha Levin @ 2011-05-06 11:24 ` Sasha Levin 2011-05-06 11:56 ` Ingo Molnar 2011-05-07 9:13 ` [PATCH 1/3] kvm tools: Introduce " Pekka Enberg 2 siblings, 1 reply; 7+ messages in thread From: Sasha Levin @ 2011-05-06 11:24 UTC (permalink / raw) To: penberg; +Cc: mingo, asias.hejun, gorcunov, prasadjoshi124, kvm, Sasha Levin Instead of using static IRQ/device data, register the device upon initialization and use the assign parameters when issuing IRQs. Clean up static definitions of IRQs. Signed-off-by: Sasha Levin <levinsasha928@gmail.com> --- tools/kvm/include/kvm/virtio-pci-dev.h | 21 ------------- tools/kvm/virtio/blk.c | 17 +++++++---- tools/kvm/virtio/console.c | 40 ++++++++++++++----------- tools/kvm/virtio/net.c | 40 ++++++++++++++----------- tools/kvm/virtio/rng.c | 50 ++++++++++++++++++-------------- 5 files changed, 85 insertions(+), 83 deletions(-) diff --git a/tools/kvm/include/kvm/virtio-pci-dev.h b/tools/kvm/include/kvm/virtio-pci-dev.h index 431289d..d2274d0 100644 --- a/tools/kvm/include/kvm/virtio-pci-dev.h +++ b/tools/kvm/include/kvm/virtio-pci-dev.h @@ -16,25 +16,4 @@ #define PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE 0x0003 #define PCI_SUBSYSTEM_ID_VIRTIO_RNG 0x0004 -enum { - PCI_VIRTIO_BLK_DEVNUM = 10, - PCI_VIRTIO_CONSOLE_DEVNUM = 2, - PCI_VIRTIO_NET_DEVNUM = 3, - PCI_VIRTIO_RNG_DEVNUM = 4, -}; - -enum { - VIRTIO_BLK_PIN = 1, - VIRTIO_CONSOLE_PIN = 2, - VIRTIO_NET_PIN = 3, - VIRTIO_RNG_PIN = 4, -}; - -enum { - VIRTIO_RNG_IRQ = 11, - VIRTIO_CONSOLE_IRQ = 13, - VIRTIO_NET_IRQ = 14, - VIRTIO_BLK_IRQ = 15, -}; - #endif /* VIRTIO_PCI_DEV_H_ */ diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c index 5a73dd6..accfc3e 100644 --- a/tools/kvm/virtio/blk.c +++ b/tools/kvm/virtio/blk.c @@ -2,7 +2,7 @@ #include "kvm/virtio-pci.h" #include "kvm/virtio-pci-dev.h" - +#include "kvm/irq.h" #include "kvm/disk-image.h" #include "kvm/virtio.h" #include "kvm/ioport.h" @@ -103,7 +103,7 @@ static bool virtio_blk_pci_io_in(struct kvm *self, u16 port, void *data, int siz break; case VIRTIO_PCI_ISR: ioport__write8(data, 0x1); - kvm__irq_line(self, VIRTIO_BLK_IRQ + bdev->idx, 0); + kvm__irq_line(self, bdev->pci_device.irq_line, 0); break; case VIRTIO_MSI_CONFIG_VECTOR: ioport__write16(data, bdev->config_vector); @@ -167,7 +167,7 @@ static void virtio_blk_do_io(struct kvm *kvm, void *param) while (virt_queue__available(vq)) virtio_blk_do_io_request(kvm, bdev, vq); - kvm__irq_line(kvm, VIRTIO_BLK_IRQ + bdev->idx, 1); + kvm__irq_line(kvm, bdev->pci_device.irq_line, 1); } static bool virtio_blk_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count) @@ -257,6 +257,7 @@ static int virtio_blk_find_empty_dev(void) void virtio_blk__init(struct kvm *self, struct disk_image *disk) { u16 blk_dev_base_addr; + u8 dev, pin, line; struct blk_dev *bdev; int new_dev_idx; @@ -291,12 +292,16 @@ void virtio_blk__init(struct kvm *self, struct disk_image *disk) .subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET, .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_BLK, .bar[0] = blk_dev_base_addr | PCI_BASE_ADDRESS_SPACE_IO, - .irq_pin = VIRTIO_BLK_PIN, - .irq_line = VIRTIO_BLK_IRQ + new_dev_idx, }, }; - pci__register(&bdev->pci_device, PCI_VIRTIO_BLK_DEVNUM + new_dev_idx); + if (irq__register_device(PCI_DEVICE_ID_VIRTIO_BLK, &dev, &pin, &line) < 0) + return; + + bdev->pci_device.irq_pin = pin; + bdev->pci_device.irq_line = line; + + pci__register(&bdev->pci_device, dev); ioport__register(blk_dev_base_addr, &virtio_blk_io_ops, IOPORT_VIRTIO_BLK_SIZE); } diff --git a/tools/kvm/virtio/console.c b/tools/kvm/virtio/console.c index 02272b4..188227b 100644 --- a/tools/kvm/virtio/console.c +++ b/tools/kvm/virtio/console.c @@ -10,6 +10,7 @@ #include "kvm/kvm.h" #include "kvm/pci.h" #include "kvm/threadpool.h" +#include "kvm/irq.h" #include <linux/virtio_console.h> #include <linux/virtio_ring.h> @@ -28,6 +29,17 @@ #define VIRTIO_CONSOLE_RX_QUEUE 0 #define VIRTIO_CONSOLE_TX_QUEUE 1 +static struct pci_device_header virtio_console_pci_device = { + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, + .device_id = PCI_DEVICE_ID_VIRTIO_CONSOLE, + .header_type = PCI_HEADER_TYPE_NORMAL, + .revision_id = 0, + .class = 0x078000, + .subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET, + .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE, + .bar[0] = IOPORT_VIRTIO_CONSOLE | PCI_BASE_ADDRESS_SPACE_IO, +}; + struct con_dev { pthread_mutex_t mutex; @@ -73,7 +85,7 @@ static void virtio_console__inject_interrupt_callback(struct kvm *self, void *pa head = virt_queue__get_iov(vq, iov, &out, &in, self); len = term_getc_iov(CONSOLE_VIRTIO, iov, in); virt_queue__set_used_elem(vq, head, len); - kvm__irq_line(self, VIRTIO_CONSOLE_IRQ, 1); + kvm__irq_line(self, virtio_console_pci_device.irq_line, 1); } mutex_unlock(&cdev.mutex); @@ -128,7 +140,7 @@ static bool virtio_console_pci_io_in(struct kvm *self, u16 port, void *data, int break; case VIRTIO_PCI_ISR: ioport__write8(data, 0x1); - kvm__irq_line(self, VIRTIO_CONSOLE_IRQ, 0); + kvm__irq_line(self, virtio_console_pci_device.irq_line, 0); break; case VIRTIO_MSI_CONFIG_VECTOR: ioport__write16(data, cdev.config_vector); @@ -158,7 +170,7 @@ static void virtio_console_handle_callback(struct kvm *self, void *param) virt_queue__set_used_elem(vq, head, len); } - kvm__irq_line(self, VIRTIO_CONSOLE_IRQ, 1); + kvm__irq_line(self, virtio_console_pci_device.irq_line, 1); } static bool virtio_console_pci_io_out(struct kvm *self, u16 port, void *data, int size, u32 count) @@ -222,21 +234,15 @@ static struct ioport_operations virtio_console_io_ops = { .io_out = virtio_console_pci_io_out, }; -static struct pci_device_header virtio_console_pci_device = { - .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, - .device_id = PCI_DEVICE_ID_VIRTIO_CONSOLE, - .header_type = PCI_HEADER_TYPE_NORMAL, - .revision_id = 0, - .class = 0x078000, - .subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET, - .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE, - .bar[0] = IOPORT_VIRTIO_CONSOLE | PCI_BASE_ADDRESS_SPACE_IO, - .irq_pin = VIRTIO_CONSOLE_PIN, - .irq_line = VIRTIO_CONSOLE_IRQ, -}; - void virtio_console__init(struct kvm *self) { - pci__register(&virtio_console_pci_device, PCI_VIRTIO_CONSOLE_DEVNUM); + u8 dev, line, pin; + + if (irq__register_device(PCI_DEVICE_ID_VIRTIO_CONSOLE, &dev, &pin, &line) < 0) + return; + + virtio_console_pci_device.irq_pin = pin; + virtio_console_pci_device.irq_line = line; + pci__register(&virtio_console_pci_device, dev); ioport__register(IOPORT_VIRTIO_CONSOLE, &virtio_console_io_ops, IOPORT_VIRTIO_CONSOLE_SIZE); } diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c index df69ab3..99ea22a 100644 --- a/tools/kvm/virtio/net.c +++ b/tools/kvm/virtio/net.c @@ -8,6 +8,7 @@ #include "kvm/util.h" #include "kvm/kvm.h" #include "kvm/pci.h" +#include "kvm/irq.h" #include <linux/virtio_net.h> #include <linux/if_tun.h> @@ -26,6 +27,17 @@ #define VIRTIO_NET_RX_QUEUE 0 #define VIRTIO_NET_TX_QUEUE 1 +static struct pci_device_header virtio_net_pci_device = { + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, + .device_id = PCI_DEVICE_ID_VIRTIO_NET, + .header_type = PCI_HEADER_TYPE_NORMAL, + .revision_id = 0, + .class = 0x020000, + .subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET, + .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_NET, + .bar[0] = IOPORT_VIRTIO_NET | PCI_BASE_ADDRESS_SPACE_IO, +}; + struct net_device { pthread_mutex_t mutex; @@ -89,7 +101,7 @@ static void *virtio_net_rx_thread(void *p) len = readv(net_device.tap_fd, iov, in); virt_queue__set_used_elem(vq, head, len); /* We should interrupt guest right now, otherwise latency is huge. */ - kvm__irq_line(self, VIRTIO_NET_IRQ, 1); + kvm__irq_line(self, virtio_net_pci_device.irq_line, 1); } } @@ -123,7 +135,7 @@ static void *virtio_net_tx_thread(void *p) virt_queue__set_used_elem(vq, head, len); } - kvm__irq_line(self, VIRTIO_NET_IRQ, 1); + kvm__irq_line(self, virtio_net_pci_device.irq_line, 1); } pthread_exit(NULL); @@ -176,7 +188,7 @@ static bool virtio_net_pci_io_in(struct kvm *self, u16 port, void *data, int siz break; case VIRTIO_PCI_ISR: ioport__write8(data, 0x1); - kvm__irq_line(self, VIRTIO_NET_IRQ, 0); + kvm__irq_line(self, virtio_net_pci_device.irq_line, 0); break; case VIRTIO_MSI_CONFIG_VECTOR: ioport__write16(data, net_device.config_vector); @@ -266,19 +278,6 @@ static struct ioport_operations virtio_net_io_ops = { .io_out = virtio_net_pci_io_out, }; -static struct pci_device_header virtio_net_pci_device = { - .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, - .device_id = PCI_DEVICE_ID_VIRTIO_NET, - .header_type = PCI_HEADER_TYPE_NORMAL, - .revision_id = 0, - .class = 0x020000, - .subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET, - .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_NET, - .bar[0] = IOPORT_VIRTIO_NET | PCI_BASE_ADDRESS_SPACE_IO, - .irq_pin = VIRTIO_NET_PIN, - .irq_line = VIRTIO_NET_IRQ, -}; - static bool virtio_net__tap_init(const struct virtio_net_parameters *params) { int sock = socket(AF_INET, SOCK_STREAM, 0); @@ -380,7 +379,14 @@ static void virtio_net__io_thread_init(struct kvm *self) void virtio_net__init(const struct virtio_net_parameters *params) { if (virtio_net__tap_init(params)) { - pci__register(&virtio_net_pci_device, PCI_VIRTIO_NET_DEVNUM); + u8 dev, line, pin; + + if (irq__register_device(PCI_DEVICE_ID_VIRTIO_NET, &dev, &pin, &line) < 0) + return; + + virtio_net_pci_device.irq_pin = pin; + virtio_net_pci_device.irq_line = line; + pci__register(&virtio_net_pci_device, dev); ioport__register(IOPORT_VIRTIO_NET, &virtio_net_io_ops, IOPORT_VIRTIO_NET_SIZE); virtio_net__io_thread_init(params->self); diff --git a/tools/kvm/virtio/rng.c b/tools/kvm/virtio/rng.c index f6034e4..d355cf8 100644 --- a/tools/kvm/virtio/rng.c +++ b/tools/kvm/virtio/rng.c @@ -11,6 +11,7 @@ #include "kvm/kvm.h" #include "kvm/pci.h" #include "kvm/threadpool.h" +#include "kvm/irq.h" #include <linux/virtio_ring.h> #include <linux/virtio_rng.h> @@ -23,15 +24,26 @@ #define NUM_VIRT_QUEUES 1 #define VIRTIO_RNG_QUEUE_SIZE 128 +static struct pci_device_header virtio_rng_pci_device = { + .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, + .device_id = PCI_DEVICE_ID_VIRTIO_RNG, + .header_type = PCI_HEADER_TYPE_NORMAL, + .revision_id = 0, + .class = 0x010000, + .subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET, + .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_RNG, + .bar[0] = IOPORT_VIRTIO_RNG | PCI_BASE_ADDRESS_SPACE_IO, +}; + struct rng_dev { - u8 status; - u16 config_vector; - int fd; + u8 status; + u16 config_vector; + int fd; /* virtio queue */ - u16 queue_selector; - struct virt_queue vqs[NUM_VIRT_QUEUES]; - void *jobs[NUM_VIRT_QUEUES]; + u16 queue_selector; + struct virt_queue vqs[NUM_VIRT_QUEUES]; + void *jobs[NUM_VIRT_QUEUES]; }; static struct rng_dev rdev; @@ -61,7 +73,7 @@ static bool virtio_rng_pci_io_in(struct kvm *kvm, u16 port, void *data, int size break; case VIRTIO_PCI_ISR: ioport__write8(data, 0x1); - kvm__irq_line(kvm, VIRTIO_RNG_IRQ, 0); + kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, 0); break; case VIRTIO_MSI_CONFIG_VECTOR: ioport__write16(data, rdev.config_vector); @@ -94,7 +106,7 @@ static void virtio_rng_do_io(struct kvm *kvm, void *param) while (virt_queue__available(vq)) { virtio_rng_do_io_request(kvm, vq); - kvm__irq_line(kvm, VIRTIO_RNG_IRQ, 1); + kvm__irq_line(kvm, virtio_rng_pci_device.irq_line, 1); } } @@ -151,26 +163,20 @@ static struct ioport_operations virtio_rng_io_ops = { .io_out = virtio_rng_pci_io_out, }; -static struct pci_device_header virtio_rng_pci_device = { - .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET, - .device_id = PCI_DEVICE_ID_VIRTIO_RNG, - .header_type = PCI_HEADER_TYPE_NORMAL, - .revision_id = 0, - .class = 0x010000, - .subsys_vendor_id = PCI_SUBSYSTEM_VENDOR_ID_REDHAT_QUMRANET, - .subsys_id = PCI_SUBSYSTEM_ID_VIRTIO_RNG, - .bar[0] = IOPORT_VIRTIO_RNG | PCI_BASE_ADDRESS_SPACE_IO, - .irq_pin = VIRTIO_RNG_PIN, - .irq_line = VIRTIO_RNG_IRQ, -}; - void virtio_rng__init(struct kvm *kvm) { + u8 pin, line, dev; + rdev.fd = open("/dev/urandom", O_RDONLY); if (rdev.fd < 0) die("Failed initializing RNG"); - pci__register(&virtio_rng_pci_device, PCI_VIRTIO_RNG_DEVNUM); + if (irq__register_device(PCI_DEVICE_ID_VIRTIO_RNG, &dev, &pin, &line) < 0) + return; + + virtio_rng_pci_device.irq_pin = pin; + virtio_rng_pci_device.irq_line = line; + pci__register(&virtio_rng_pci_device, dev); ioport__register(IOPORT_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE); } -- 1.7.5.rc3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry 2011-05-06 11:24 ` [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry Sasha Levin @ 2011-05-06 11:56 ` Ingo Molnar 2011-05-06 16:50 ` Sasha Levin 0 siblings, 1 reply; 7+ messages in thread From: Ingo Molnar @ 2011-05-06 11:56 UTC (permalink / raw) To: Sasha Levin; +Cc: penberg, asias.hejun, gorcunov, prasadjoshi124, kvm * Sasha Levin <levinsasha928@gmail.com> wrote: > + bdev->pci_device.irq_pin = pin; > + bdev->pci_device.irq_line = line; One small remaining naming inconsistency caught my eyes. The generic convention should be something like: - structure names should be along the 'struct xyz_device' scheme - structure field names should be 'xyz_dev' - variable names within xyz driver's .c file should be 'xdev', but 'xyz_dev' is OK too, especially if used in some other file) In that sense, the above should be: bdev->pci_dev.irq_pin = pin; bdev->pci_dev.irq_line = line; This could be fixed in a followup patch - and there's more of the same inconsistency in other driver files as well. If such details are sorted out early on in a project's lifetime it will be applied in a very natural way as the code grows. Thanks, Ingo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry 2011-05-06 11:56 ` Ingo Molnar @ 2011-05-06 16:50 ` Sasha Levin 2011-05-06 19:16 ` Ingo Molnar 0 siblings, 1 reply; 7+ messages in thread From: Sasha Levin @ 2011-05-06 16:50 UTC (permalink / raw) To: Ingo Molnar; +Cc: penberg, asias.hejun, gorcunov, prasadjoshi124, kvm On Fri, 2011-05-06 at 13:56 +0200, Ingo Molnar wrote: > * Sasha Levin <levinsasha928@gmail.com> wrote: > > > + bdev->pci_device.irq_pin = pin; > > + bdev->pci_device.irq_line = line; > > One small remaining naming inconsistency caught my eyes. The generic convention > should be something like: > > - structure names should be along the 'struct xyz_device' scheme > > - structure field names should be 'xyz_dev' > > - variable names within xyz driver's .c file should be 'xdev', > but 'xyz_dev' is OK too, especially if used in some other file) > > In that sense, the above should be: > > bdev->pci_dev.irq_pin = pin; > bdev->pci_dev.irq_line = line; > > This could be fixed in a followup patch - and there's more of the same > inconsistency in other driver files as well. > > If such details are sorted out early on in a project's lifetime it will be > applied in a very natural way as the code grows. The struct name there actually refers to a PCI header of a device, and not an actual device. I'll rename it to pci_hdr instead of making it pci_dev, since it looks more confusing than desirable. -- Sasha. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry 2011-05-06 16:50 ` Sasha Levin @ 2011-05-06 19:16 ` Ingo Molnar 0 siblings, 0 replies; 7+ messages in thread From: Ingo Molnar @ 2011-05-06 19:16 UTC (permalink / raw) To: Sasha Levin; +Cc: penberg, asias.hejun, gorcunov, prasadjoshi124, kvm * Sasha Levin <levinsasha928@gmail.com> wrote: > On Fri, 2011-05-06 at 13:56 +0200, Ingo Molnar wrote: > > * Sasha Levin <levinsasha928@gmail.com> wrote: > > > > > + bdev->pci_device.irq_pin = pin; > > > + bdev->pci_device.irq_line = line; > > > > One small remaining naming inconsistency caught my eyes. The generic convention > > should be something like: > > > > - structure names should be along the 'struct xyz_device' scheme > > > > - structure field names should be 'xyz_dev' > > > > - variable names within xyz driver's .c file should be 'xdev', > > but 'xyz_dev' is OK too, especially if used in some other file) > > > > In that sense, the above should be: > > > > bdev->pci_dev.irq_pin = pin; > > bdev->pci_dev.irq_line = line; > > > > This could be fixed in a followup patch - and there's more of the same > > inconsistency in other driver files as well. > > > > If such details are sorted out early on in a project's lifetime it will be > > applied in a very natural way as the code grows. > > The struct name there actually refers to a PCI header of a device, and > not an actual device. > > I'll rename it to pci_hdr instead of making it pci_dev, since it looks > more confusing than desirable. Yeah, indeed that would be good, it certainly confused me! :-) Thanks, Ingo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] kvm tools: Introduce IRQ registry 2011-05-06 11:24 [PATCH 1/3] kvm tools: Introduce IRQ registry Sasha Levin 2011-05-06 11:24 ` [PATCH 2/3] kvm tools: Dynamically add devices when creating mptable Sasha Levin 2011-05-06 11:24 ` [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry Sasha Levin @ 2011-05-07 9:13 ` Pekka Enberg 2 siblings, 0 replies; 7+ messages in thread From: Pekka Enberg @ 2011-05-07 9:13 UTC (permalink / raw) To: Sasha Levin; +Cc: mingo, asias.hejun, gorcunov, prasadjoshi124, kvm On Fri, 6 May 2011, Sasha Levin wrote: > +int irq__register_device(u32 dev, u8 *num, u8 *pin, u8 *line) > +{ > + struct pci_dev *node; > + > + node = search(&pci_tree, dev); > + > + if (!node) { > + /* We haven't found a node - First device of it's kind */ > + node = malloc(sizeof(*node)); > + if (node == NULL) > + return -1; > + This is never free'd: # KVM session ended normally. ==29360== ==29360== HEAP SUMMARY: ==29360== in use at exit: 1,008 bytes in 8 blocks ==29360== total heap usage: 23 allocs, 15 frees, 33,565,229 bytes allocated ==29360== ==29360== 24 bytes in 1 blocks are possibly lost in loss record 2 of 7 ==29360== at 0x4C275D8: malloc (vg_replace_malloc.c:236) ==29360== by 0x40849F: irq__register_device (irq.c:87) ==29360== by 0x404E4A: virtio_blk__init (blk.c:298) ==29360== by 0x40766E: kvm_cmd_run (kvm-run.c:388) ==29360== by 0x404636: main (main.c:16) ==29360== ==29360== 576 bytes in 2 blocks are possibly lost in loss record 7 of 7 ==29360== at 0x4C268FC: calloc (vg_replace_malloc.c:467) ==29360== by 0x4012455: _dl_allocate_tls (dl-tls.c:300) ==29360== by 0x503D728: pthread_create@@GLIBC_2.2.5 (allocatestack.c:561) ==29360== by 0x4082A1: thread_pool__init (threadpool.c:121) ==29360== by 0x407827: kvm_cmd_run (kvm-run.c:447) ==29360== by 0x404636: main (main.c:16) ==29360== ==29360== LEAK SUMMARY: ==29360== definitely lost: 0 bytes in 0 blocks ==29360== indirectly lost: 0 bytes in 0 blocks ==29360== possibly lost: 600 bytes in 3 blocks ==29360== still reachable: 408 bytes in 5 blocks ==29360== suppressed: 0 bytes in 0 blocks ==29360== Reachable blocks (those to which a pointer was found) are not shown. ==29360== To see them, rerun with: --leak-check=full --show-reachable=yes ==29360== ==29360== For counts of detected and suppressed errors, rerun with: -v ==29360== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 4 from 4) While it's not an error per se, I'd really like to keep things clean when running under valgrind. Pekka ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-05-07 9:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-06 11:24 [PATCH 1/3] kvm tools: Introduce IRQ registry Sasha Levin 2011-05-06 11:24 ` [PATCH 2/3] kvm tools: Dynamically add devices when creating mptable Sasha Levin 2011-05-06 11:24 ` [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry Sasha Levin 2011-05-06 11:56 ` Ingo Molnar 2011-05-06 16:50 ` Sasha Levin 2011-05-06 19:16 ` Ingo Molnar 2011-05-07 9:13 ` [PATCH 1/3] kvm tools: Introduce " Pekka Enberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox