[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v2 2/3] virtio-pci : add a virtio-bus interf
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [RFC PATCH v2 2/3] virtio-pci : add a virtio-bus interface |
Date: |
Mon, 26 Nov 2012 08:43:42 -0600 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
address@hidden writes:
> From: KONRAD Frederic <address@hidden>
>
> This patch add a VirtioBus in the VirtIOPCIProxy structure. It creates a new
> device : "virtio-pci" which init the VirtioBus. Two callback are written :
>
> * void virtio_pci_init_cb(DeviceState *dev) to initialize the PCI
> interface
> after the VirtIODevice init, it is approximately the same as
> virtio_init_pci.
>
> * void virtio_pci_exit_cb(DeviceState *dev) to stop the ioevent before the
> VirtIODevice exit.
>
> Signed-off-by: KONRAD Frederic <address@hidden>
> ---
> hw/virtio-pci.c | 152
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/virtio-pci.h | 6 +++
> 2 files changed, 158 insertions(+)
>
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 71f4fb5..78aa5e8 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -22,6 +22,7 @@
> #include "virtio-net.h"
> #include "virtio-serial.h"
> #include "virtio-scsi.h"
> +#include "virtio-bus.h"
> #include "pci.h"
> #include "qemu-error.h"
> #include "msi.h"
> @@ -1117,15 +1118,166 @@ static TypeInfo virtio_scsi_info = {
> .instance_size = sizeof(VirtIOPCIProxy),
> .class_init = virtio_scsi_class_init,
> };
> +/* The new virtio-pci device */
> +
> +void virtio_pci_init_cb(DeviceState *dev)
> +{
> + PCIDevice *d = DO_UPCAST(PCIDevice, qdev, dev);
> + VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, d);
> + uint8_t *config;
> + uint32_t size;
> +
> + /* Put the PCI IDs */
> + switch (get_virtio_device_id(proxy->bus)) {
> +
> + case VIRTIO_ID_BLOCK:
> + pci_config_set_device_id(proxy->pci_dev.config,
> + PCI_DEVICE_ID_VIRTIO_BLOCK);
> + pci_config_set_class(proxy->pci_dev.config, PCI_CLASS_STORAGE_SCSI);
> + break;
> + default:
> + error_report("unknown device id\n");
> + break;
> +
> + }
> +
> + /* virtio_init_pci code without bindings */
> +
> + /* This should disappear */
> + proxy->vdev = proxy->bus->vdev;
> +
> + config = proxy->pci_dev.config;
> +
> + if (proxy->class_code) {
> + pci_config_set_class(config, proxy->class_code);
> + }
> + pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
> + pci_get_word(config + PCI_VENDOR_ID));
> + pci_set_word(config + PCI_SUBSYSTEM_ID, proxy->bus->vdev->device_id);
> + config[PCI_INTERRUPT_PIN] = 1;
> +
> + if (proxy->bus->vdev->nvectors &&
> + msix_init_exclusive_bar(&proxy->pci_dev, proxy->bus->vdev->nvectors,
> + 1)) {
> + proxy->bus->vdev->nvectors = 0;
> + }
> +
> + proxy->pci_dev.config_write = virtio_write_config;
> +
> + size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev)
> + + proxy->bus->vdev->config_len;
> + if (size & (size-1)) {
> + size = 1 << qemu_fls(size);
> + }
> +
> + memory_region_init_io(&proxy->bar, &virtio_pci_config_ops, proxy,
> + "virtio-pci", size);
> + pci_register_bar(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
> + &proxy->bar);
> +
> + if (!kvm_has_many_ioeventfds()) {
> + proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
> + }
> +
> + /* Bind the VirtIODevice to the VirtioBus. */
> + virtio_bus_bind_device(proxy->bus);
> +
> + proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
> + proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE;
> + /* Should be modified */
> + proxy->host_features = proxy->bus->vdev->get_features(proxy->bus->vdev,
> +
> proxy->host_features);
> +}
> +
> +void virtio_pci_exit_cb(DeviceState *dev)
> +{
> + PCIDevice *d = DO_UPCAST(PCIDevice, qdev, dev);
> + VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, d);
> + /* Put the PCI IDs */
> + pci_config_set_device_id(proxy->pci_dev.config, 0x0000);
> + pci_config_set_class(proxy->pci_dev.config, PCI_CLASS_OTHERS);
> +
> + virtio_pci_stop_ioeventfd(proxy);
> +}
> +
> +static const struct VirtioBusInfo virtio_bus_info = {
> + .init_cb = virtio_pci_init_cb,
> + .exit_cb = virtio_pci_exit_cb,
> +
> + .virtio_bindings = {
> + .notify = virtio_pci_notify,
> + .save_config = virtio_pci_save_config,
> + .load_config = virtio_pci_load_config,
> + .save_queue = virtio_pci_save_queue,
> + .load_queue = virtio_pci_load_queue,
> + .get_features = virtio_pci_get_features,
> + .query_guest_notifiers = virtio_pci_query_guest_notifiers,
> + .set_host_notifier = virtio_pci_set_host_notifier,
> + .set_guest_notifiers = virtio_pci_set_guest_notifiers,
> + .vmstate_change = virtio_pci_vmstate_change,
> + }
> +};
So this looks wrong.
The interactions should roughly look like:
(1) VirtioDevice only interacts with VirtioBus through it's class
methods. 'notify' is a method.
(2) Since VirtioBus is a descrete object, it needs to interact with
whatever the actual bus is. There are two ways to do this:
(a) Each bus-type can sub-class VirtioBus and implement each method
using a private interface. This is probably the easiest
approach because you can just give VirtioPCIBus a pointer to the
PCIDevice, and then implement the methods within VirtioPCIBus.
(b) There can be only one VirtioBus object type with a well defined
proxy interface. This should basically mirror the VirtioBus
class methods. VirtioPCI can then implement this proxy
interface.
Seeing how the code is turning out, I think 2.a would require the least
amount of coding.
> +
> +static int virtiopci_qdev_init(PCIDevice *dev)
> +{
> + VirtIOPCIProxy *s = DO_UPCAST(VirtIOPCIProxy, pci_dev, dev);
> + DeviceState *qdev = DEVICE(dev);
> +
> + /* create a virtio-bus and attach virtio-pci to it */
> + s->bus = virtio_bus_new(qdev, &virtio_bus_info);
You basically end up with virtio_pci_bus_new() here and you pass 'dev'
to it. VirtioPCIBus is a 'friend' to VirtioPCI so it can access it's
private data.
> +
> + return 0;
> +}
> +
Regards,
Anthony Liguori
> +static Property virtiopci_properties[] = {
> + /* TODO : Add the correct properties */
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtiopci_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> + PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
> +
> + pc->init = virtiopci_qdev_init;
> + pc->exit = virtio_exit_pci;
> + pc->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> + pc->revision = VIRTIO_PCI_ABI_VERSION;
> + pc->class_id = PCI_CLASS_OTHERS;
> + /* TODO : Add the correct device information below */
> + /* pc->exit =
> + * pc->device_id =
> + * pc->subsystem_vendor_id =
> + * pc->subsystem_id =
> + * dc->reset =
> + * dc->vmsd =
> + */
> + dc->props = virtiopci_properties;
> + dc->desc = "virtio-pci transport.";
> +}
> +
> +static const TypeInfo virtio_pci_info = {
> + .name = "virtio-pci",
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(VirtIOPCIProxy),
> + .class_init = virtiopci_class_init,
> +};
> +
> +
> +/************************************/
> +
>
> static void virtio_pci_register_types(void)
> {
> + /* This should disappear */
> type_register_static(&virtio_blk_info);
> type_register_static(&virtio_net_info);
> type_register_static(&virtio_serial_info);
> type_register_static(&virtio_balloon_info);
> type_register_static(&virtio_scsi_info);
> type_register_static(&virtio_rng_info);
> + /* new virtio-pci device */
> + type_register_static(&virtio_pci_info);
> }
>
> type_init(virtio_pci_register_types)
> diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
> index b58d9a2..a7a9847 100644
> --- a/hw/virtio-pci.h
> +++ b/hw/virtio-pci.h
> @@ -20,6 +20,7 @@
> #include "virtio-rng.h"
> #include "virtio-serial.h"
> #include "virtio-scsi.h"
> +#include "virtio-bus.h"
>
> /* Performance improves when virtqueue kick processing is decoupled from the
> * vcpu thread using ioeventfd for some devices. */
> @@ -33,6 +34,7 @@ typedef struct {
>
> typedef struct {
> PCIDevice pci_dev;
> + /* This should be removed */
> VirtIODevice *vdev;
> MemoryRegion bar;
> uint32_t flags;
> @@ -51,10 +53,14 @@ typedef struct {
> bool ioeventfd_disabled;
> bool ioeventfd_started;
> VirtIOIRQFD *vector_irqfd;
> + /* VirtIOBus to connect the VirtIODevice */
> + VirtioBus *bus;
> } VirtIOPCIProxy;
>
> void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
> void virtio_pci_reset(DeviceState *d);
> +void virtio_pci_init_cb(DeviceState *dev);
> +void virtio_pci_exit_cb(DeviceState *dev);
>
> /* Virtio ABI version, if we increment this, we break the guest driver. */
> #define VIRTIO_PCI_ABI_VERSION 0
> --
> 1.7.11.7
Re: [Qemu-devel] [RFC PATCH v2 2/3] virtio-pci : add a virtio-bus interface,
Anthony Liguori <=
[Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus., fred . konrad, 2012/11/22
Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus., Stefan Hajnoczi, 2012/11/23
Re: [Qemu-devel] [RFC PATCH v2 1/3] virtio-bus : Introduce VirtioBus., Andreas Färber, 2012/11/24