[RFC,v2,3/8] virtio_iommu: add skeleton

This patchs adds the skeleton for the virtio-iommu device.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/virtio/Makefile.objs          |   1 +
 hw/virtio/virtio-iommu.c         | 247 +++++++++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-iommu.h |  60 ++++++++++
 3 files changed, 308 insertions(+)
 create mode 100644 hw/virtio/virtio-iommu.c
 create mode 100644 include/hw/virtio/virtio-iommu.h

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@redhat.com]
> Sent: Wednesday, June 07, 2017 9:31 PM
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org; jean-
> philippe.brucker@arm.com
> Cc: will.deacon@arm.com; robin.murphy@arm.com; kevin.tian@intel.com;
> marc.zyngier@arm.com; christoffer.dall@linaro.org; drjones@redhat.com;
> wei@redhat.com; tn@semihalf.com; Bharat Bhushan
> <bharat.bhushan@nxp.com>
> Subject: [RFC v2 3/8] virtio_iommu: add skeleton
> 
> This patchs adds the skeleton for the virtio-iommu device.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/virtio/Makefile.objs          |   1 +
>  hw/virtio/virtio-iommu.c         | 247
> +++++++++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-iommu.h |  60 ++++++++++
>  3 files changed, 308 insertions(+)
>  create mode 100644 hw/virtio/virtio-iommu.c
>  create mode 100644 include/hw/virtio/virtio-iommu.h
> 
> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> index 765d363..8967a4a 100644
> --- a/hw/virtio/Makefile.objs
> +++ b/hw/virtio/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-y += virtio-mmio.o
> 
>  obj-y += virtio.o virtio-balloon.o
>  obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
> +obj-$(CONFIG_LINUX) += virtio-iommu.o
>  obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
>  obj-y += virtio-crypto.o
>  obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> new file mode 100644
> index 0000000..86129ef
> --- /dev/null
> +++ b/hw/virtio/virtio-iommu.c
> @@ -0,0 +1,247 @@
> +/*
> + * virtio-iommu device
> + *
> + * Copyright (c) 2017 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/iov.h"
> +#include "qemu-common.h"
> +#include "hw/virtio/virtio.h"
> +#include "sysemu/kvm.h"
> +#include "qapi-event.h"
> +#include "trace.h"
> +
> +#include "standard-headers/linux/virtio_ids.h"
> +#include <linux/virtio_iommu.h>
> +
> +#include "hw/virtio/virtio-bus.h"
> +#include "hw/virtio/virtio-access.h"
> +#include "hw/virtio/virtio-iommu.h"
> +
> +/* Max size */
> +#define VIOMMU_DEFAULT_QUEUE_SIZE 256
> +
> +static int virtio_iommu_handle_attach(VirtIOIOMMU *s,
> +                                      struct iovec *iov,
> +                                      unsigned int iov_cnt)
> +{
> +    return -ENOENT;
> +}
> +static int virtio_iommu_handle_detach(VirtIOIOMMU *s,
> +                                      struct iovec *iov,
> +                                      unsigned int iov_cnt)
> +{
> +    return -ENOENT;
> +}
> +static int virtio_iommu_handle_map(VirtIOIOMMU *s,
> +                                   struct iovec *iov,
> +                                   unsigned int iov_cnt)
> +{
> +    return -ENOENT;
> +}
> +static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
> +                                     struct iovec *iov,
> +                                     unsigned int iov_cnt)
> +{
> +    return -ENOENT;
> +}
> +
> +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue
> *vq)
> +{
> +    VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> +    VirtQueueElement *elem;
> +    struct virtio_iommu_req_head head;
> +    struct virtio_iommu_req_tail tail;
> +    unsigned int iov_cnt;
> +    struct iovec *iov;
> +    size_t sz;
> +
> +    for (;;) {
> +        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> +        if (!elem) {
> +            return;
> +        }
> +
> +        if (iov_size(elem->in_sg, elem->in_num) < sizeof(tail) ||
> +            iov_size(elem->out_sg, elem->out_num) < sizeof(head)) {
> +            virtio_error(vdev, "virtio-iommu erroneous head or tail");
> +            virtqueue_detach_element(vq, elem, 0);
> +            g_free(elem);
> +            break;
> +        }
> +
> +        iov_cnt = elem->out_num;
> +        iov = g_memdup(elem->out_sg, sizeof(struct iovec) * elem-
> >out_num);
> +        sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
> +        if (sz != sizeof(head)) {
> +            tail.status = VIRTIO_IOMMU_S_UNSUPP;
> +        }
> +        qemu_mutex_lock(&s->mutex);
> +        switch (head.type) {
> +        case VIRTIO_IOMMU_T_ATTACH:
> +            tail.status = virtio_iommu_handle_attach(s, iov, iov_cnt);
> +            break;
> +        case VIRTIO_IOMMU_T_DETACH:
> +            tail.status = virtio_iommu_handle_detach(s, iov, iov_cnt);
> +            break;
> +        case VIRTIO_IOMMU_T_MAP:
> +            tail.status = virtio_iommu_handle_map(s, iov, iov_cnt);
> +            break;
> +        case VIRTIO_IOMMU_T_UNMAP:
> +            tail.status = virtio_iommu_handle_unmap(s, iov, iov_cnt);
> +            break;
> +        default:
> +            tail.status = VIRTIO_IOMMU_S_UNSUPP;
> +        }
> +        qemu_mutex_unlock(&s->mutex);
> +
> +        sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
> +                          &tail, sizeof(tail));
> +        assert(sz == sizeof(tail));
> +
> +        virtqueue_push(vq, elem, sizeof(tail));
> +        virtio_notify(vdev, vq);
> +        g_free(elem);
> +    }
> +}
> +
> +static void virtio_iommu_get_config(VirtIODevice *vdev, uint8_t
> *config_data)
> +{
> +    VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
> +
> +    memcpy(config_data, &dev->config, sizeof(struct virtio_iommu_config));
> +}
> +
> +static void virtio_iommu_set_config(VirtIODevice *vdev,
> +                                      const uint8_t *config_data)
> +{
> +    VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
> +    struct virtio_iommu_config config;
> +
> +    memcpy(&config, config_data, sizeof(struct virtio_iommu_config));
> +
> +    dev->config.page_sizes = le64_to_cpu(config.page_sizes);
> +    dev->config.input_range.end = le64_to_cpu(config.input_range.end);
> +}
> +
> +static uint64_t virtio_iommu_get_features(VirtIODevice *vdev, uint64_t f,
> +                                            Error **errp)
> +{
> +    VirtIOIOMMU *dev = VIRTIO_IOMMU(vdev);
> +    f |= dev->host_features;
> +    virtio_add_feature(&f, VIRTIO_RING_F_EVENT_IDX);
> +    virtio_add_feature(&f, VIRTIO_RING_F_INDIRECT_DESC);
> +    virtio_add_feature(&f, VIRTIO_IOMMU_F_INPUT_RANGE);
> +    return f;
> +}
> +
> +static int virtio_iommu_post_load_device(void *opaque, int version_id)
> +{
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_virtio_iommu_device = {
> +    .name = "virtio-iommu-device",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = virtio_iommu_post_load_device,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
> +
> +    virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
> +                sizeof(struct virtio_iommu_config));
> +
> +    s->vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
> +                             virtio_iommu_handle_command);
> +
> +    s->config.page_sizes = ~((1ULL << 12) - 1);

This is hardcoded to 4K, Should this be aligned to Host-page size ?

Thanks
-Bharat

> +    s->config.input_range.end = -1UL;
> +}
> +
> +static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +
> +    virtio_cleanup(vdev);
> +}
> +
> +static void virtio_iommu_device_reset(VirtIODevice *vdev)
> +{
> +}
> +
> +static void virtio_iommu_set_status(VirtIODevice *vdev, uint8_t status)
> +{
> +}
> +
> +static void virtio_iommu_instance_init(Object *obj)
> +{
> +}
> +
> +static const VMStateDescription vmstate_virtio_iommu = {
> +    .name = "virtio-iommu",
> +    .minimum_version_id = 1,
> +    .version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_VIRTIO_DEVICE,
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +static Property virtio_iommu_properties[] = {
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtio_iommu_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +
> +    dc->props = virtio_iommu_properties;
> +    dc->vmsd = &vmstate_virtio_iommu;
> +
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +    vdc->realize = virtio_iommu_device_realize;
> +    vdc->unrealize = virtio_iommu_device_unrealize;
> +    vdc->reset = virtio_iommu_device_reset;
> +    vdc->get_config = virtio_iommu_get_config;
> +    vdc->set_config = virtio_iommu_set_config;
> +    vdc->get_features = virtio_iommu_get_features;
> +    vdc->set_status = virtio_iommu_set_status;
> +    vdc->vmsd = &vmstate_virtio_iommu_device;
> +}
> +
> +static const TypeInfo virtio_iommu_info = {
> +    .name = TYPE_VIRTIO_IOMMU,
> +    .parent = TYPE_VIRTIO_DEVICE,
> +    .instance_size = sizeof(VirtIOIOMMU),
> +    .instance_init = virtio_iommu_instance_init,
> +    .class_init = virtio_iommu_class_init,
> +};
> +
> +static void virtio_register_types(void)
> +{
> +    type_register_static(&virtio_iommu_info);
> +}
> +
> +type_init(virtio_register_types)
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-
> iommu.h
> new file mode 100644
> index 0000000..2259413
> --- /dev/null
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -0,0 +1,60 @@
> +/*
> + * virtio-iommu device
> + *
> + * Copyright (c) 2017 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#ifndef QEMU_VIRTIO_IOMMU_H
> +#define QEMU_VIRTIO_IOMMU_H
> +
> +#include "standard-headers/linux/virtio_iommu.h"
> +#include "hw/virtio/virtio.h"
> +#include "hw/pci/pci.h"
> +
> +#define TYPE_VIRTIO_IOMMU "virtio-iommu-device"
> +#define VIRTIO_IOMMU(obj) \
> +        OBJECT_CHECK(VirtIOIOMMU, (obj), TYPE_VIRTIO_IOMMU)
> +
> +#define IOMMU_PCI_BUS_MAX      256
> +#define IOMMU_PCI_DEVFN_MAX    256
> +
> +typedef struct IOMMUDevice {
> +    void         *viommu;
> +    PCIBus       *bus;
> +    int           devfn;
> +    MemoryRegion  iommu_mr;
> +    AddressSpace  as;
> +} IOMMUDevice;
> +
> +typedef struct IOMMUPciBus {
> +    PCIBus       *bus;
> +    IOMMUDevice  *pbdev[0]; /* Parent array is sparse, so dynamically alloc
> */
> +} IOMMUPciBus;
> +
> +typedef struct VirtIOIOMMU {
> +    VirtIODevice parent_obj;
> +    VirtQueue *vq;
> +    struct virtio_iommu_config config;
> +    MemoryRegionIOMMUOps iommu_ops;
> +    uint32_t host_features;
> +    GHashTable *as_by_busptr;
> +    IOMMUPciBus *as_by_bus_num[IOMMU_PCI_BUS_MAX];
> +    GTree *address_spaces;
> +    QemuMutex mutex;
> +    GTree *devices;
> +} VirtIOIOMMU;
> +
> +#endif
> --
> 2.5.5

On 06/08/2017 12:09 PM, Bharat Bhushan wrote:
>> From: Eric Auger [mailto:eric.auger@redhat.com]
>> Sent: Wednesday, June 07, 2017 9:31 PM
>> To: eric.auger.pro@gmail.com; eric.auger@redhat.com;
>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; jean-
>> philippe.brucker@arm.com
>> Cc: will.deacon@arm.com; robin.murphy@arm.com; kevin.tian@intel.com;
>> marc.zyngier@arm.com; christoffer.dall@linaro.org; drjones@redhat.com;
>> wei@redhat.com; tn@semihalf.com; Bharat Bhushan
>> <bharat.bhushan@nxp.com>
>> Subject: [RFC v2 3/8] virtio_iommu: add skeleton
>>
>> This patchs adds the skeleton for the virtio-iommu device.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> +static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>> +{
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> +    VirtIOIOMMU *s = VIRTIO_IOMMU(dev);
>> +
>> +    virtio_init(vdev, "virtio-iommu", VIRTIO_ID_IOMMU,
>> +                sizeof(struct virtio_iommu_config));
>> +
>> +    s->vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
>> +                             virtio_iommu_handle_command);
>> +
>> +    s->config.page_sizes = ~((1ULL << 12) - 1);
> 
> This is hardcoded to 4K, Should this be aligned to Host-page size ?

I wonder if we should introduce per-address-space page sizes, to cater
for emulated and VFIO devices being managed by the same IOMMU.

For an emulated device, it seems that the page granularity can be
arbitrary, so maybe TARGET_PAGE_MASK would be more convenient. But for
VFIO, the page granularity is a property of the physical IOMMU.

In kvmtool I instantiate two virtio-iommus for vfio and virtio devices,
so the page size issue hasn't come up, but here things won't work if the
page granularity advertised in config.page_sizes is smaller than the
pIOMMU page size.

Adding address space properties is tricky because they change when
attaching devices, and I wanted to avoid this complication. In nested
mode I have to add one AS state, where the AS is active and properties
are freezed (attaching an incompatible device is then rejected). Maybe
we need to do the same for map/unmap.

A simpler solution (for me, that is), would be to put the greatest page
granularity of all VFIO devices into page_sizes, but it doesn't take
hotplug into account.

Thanks,
Jean