Virtio-loopback-rfc release:

- Polish the code, remove comments, FIXMEs ...
- Add 'print_config' as a class method
- Fix 'features_sel' read/write operations

Signed-off-by: Timos Ampelikiotis <t.ampelikiotis@virtualopensystems.com>

6 files changed, 71 insertions, 94 deletions

diff --git a/vhost_user_blk.c b/vhost_user_blk.c
index bc79557..084710b 100644
--- a/vhost_user_blk.c
+++ b/vhost_user_blk.c
@@ -76,6 +76,7 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
     }
 
     s->vhost_dev->acked_features = vdev->guest_features;
+    DBG("acked_features: 0x%lx\n", vdev->guest_features);
 
     /* FIXME: We might do not need that */
     ret = vhost_dev_prepare_inflight(s->vhost_dev, vdev);
@@ -140,7 +141,6 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
     ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
                                sizeof(struct virtio_blk_config));
     if (ret < 0) {
-        DBG("vhost_dev_get_config\n");
         return ret;
     }
 
@@ -227,8 +227,10 @@ static int vhost_user_blk_connect(VirtIODevice *vdev)
     /* Pass the new obtained features */
     global_vdev->host_features = s->vhost_dev->features;
 
-    /* Disable VIRTIO_RING_F_INDIRECT_DESC, to be supported in future release */
-    global_vdev->host_features &= ~(1ULL << VIRTIO_RING_F_INDIRECT_DESC);
+    /*
+     * The next line disables VIRTIO_RING_F_INDIRECT_DESC:
+     * global_vdev->host_features &= ~(1ULL << VIRTIO_RING_F_INDIRECT_DESC);
+     */
 
     DBG("After init global_vdev->host_features: 0x%lx\n",
                                 global_vdev->host_features);
@@ -323,14 +325,6 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
 
     DBG("vhost_user_blk_set_config(...)\n");
 
-
-    /*
-     * TODO: Disabled for the current release
-     * if (blkcfg->wce == s->blkcfg.wce) {
-     *     DBG("blkcfg->wce == s->blkcfg.wce\n");
-     *     return;
-     * }
-     */
     if (blkcfg->wce == s->blkcfg.wce) {
         DBG("blkcfg->wce == s->blkcfg.wce\n");
         return;
@@ -383,6 +377,48 @@ static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
     DBG("vhost_user_blk_set_status return successfully\n");
 }
 
+static void print_config_blk(uint8_t *config_data)
+{
+    struct virtio_blk_config *config_strct =
+        (struct virtio_blk_config *)config_data;
+
+    DBG("uint64_t capacity: %llu\n", config_strct->capacity);
+    DBG("uint32_t size_max: %u\n", config_strct->size_max);
+    DBG("uint32_t seg_max: %u\n", config_strct->seg_max);
+
+    DBG("virtio_blk_geometry:\n");
+    DBG("    uint16_t cylinders: %u\n",
+            config_strct->geometry.cylinders);
+    DBG("    uint8_t heads: %u\n",
+            config_strct->geometry.heads);
+    DBG("    uint8_t sectors: %u\n",
+            config_strct->geometry.sectors);
+
+    DBG("uint32_t blk_size: %u\n", config_strct->blk_size);
+    DBG("uint8_t physical_block_exp: %u\n",
+            config_strct->physical_block_exp);
+    DBG("uint8_t alignment_offset: %u\n",
+            config_strct->alignment_offset);
+    DBG("uint16_t min_io_size: %u\n", config_strct->min_io_size);
+    DBG("uint32_t opt_io_size: %u\n", config_strct->opt_io_size);
+    DBG("uint8_t wce: %u\n", config_strct->wce);
+    DBG("uint8_t unused: %u\n", config_strct->unused);
+    DBG("uint16_t num_queues: %u\n", config_strct->num_queues);
+    DBG("uint32_t max_discard_sectors: %u\n",
+            config_strct->max_discard_sectors);
+    DBG("uint32_t max_discard_seg: %u\n", config_strct->max_discard_seg);
+    DBG("uint32_t discard_sector_alignment: %u\n",
+            config_strct->discard_sector_alignment);
+    DBG("uint32_t max_write_zeroes_sectors: %u\n",
+            config_strct->max_write_zeroes_sectors);
+    DBG("uint32_t max_write_zeroes_seg: %u\n",
+            config_strct->max_write_zeroes_seg);
+    DBG("uint8_t write_zeroes_may_unmap: %u\n",
+            config_strct->write_zeroes_may_unmap);
+    DBG("uint8_t unused1[3]: %u\n", config_strct->unused1[0]);
+    DBG("uint8_t unused1[3]: %u\n", config_strct->unused1[1]);
+    DBG("uint8_t unused1[3]: %u\n", config_strct->unused1[2]);
+}
 
 static void virtio_dev_class_init(VirtIODevice *vdev)
 {
@@ -398,9 +434,9 @@ static void virtio_dev_class_init(VirtIODevice *vdev)
     vdev->vdev_class->set_status = vhost_user_blk_set_status;
     vdev->vdev_class->reset = vhost_user_blk_reset;
     vdev->vdev_class->update_mem_table = update_mem_table;
+    vdev->vdev_class->print_config = print_config_blk;
 }
 
-
 void vhost_user_blk_init(VirtIODevice *vdev)
 {
 
@@ -417,7 +453,6 @@ void vhost_user_blk_init(VirtIODevice *vdev)
     virtio_loopback_bus_init(vdev->vbus);
 }
 
-
 static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
     /*
@@ -427,49 +462,6 @@ static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
     DBG("vhost_user_blk_handle_output not yet implemented\n");
 }
 
-
-void print_config(uint8_t *config)
-{
-    struct virtio_blk_config *config_strct = (struct virtio_blk_config *)config;
-
-    DBG("uint64_t capacity: %llu\n", config_strct->capacity);
-    DBG("uint32_t size_max: %u\n", config_strct->size_max);
-    DBG("uint32_t seg_max: %u\n", config_strct->seg_max);
-
-    DBG("virtio_blk_geometry:\n");
-    DBG("    uint16_t cylinders: %u\n",
-            config_strct->geometry.cylinders);
-    DBG("    uint8_t heads: %u\n",
-            config_strct->geometry.heads);
-    DBG("    uint8_t sectors: %u\n",
-            config_strct->geometry.sectors);
-
-    DBG("uint32_t blk_size: %u\n", config_strct->blk_size);
-    DBG("uint8_t physical_block_exp: %u\n",
-            config_strct->physical_block_exp);
-    DBG("uint8_t alignment_offset: %u\n",
-            config_strct->alignment_offset);
-    DBG("uint16_t min_io_size: %u\n", config_strct->min_io_size);
-    DBG("uint32_t opt_io_size: %u\n", config_strct->opt_io_size);
-    DBG("uint8_t wce: %u\n", config_strct->wce);
-    DBG("uint8_t unused: %u\n", config_strct->unused);
-    DBG("uint16_t num_queues: %u\n", config_strct->num_queues);
-    DBG("uint32_t max_discard_sectors: %u\n",
-            config_strct->max_discard_sectors);
-    DBG("uint32_t max_discard_seg: %u\n", config_strct->max_discard_seg);
-    DBG("uint32_t discard_sector_alignment: %u\n",
-            config_strct->discard_sector_alignment);
-    DBG("uint32_t max_write_zeroes_sectors: %u\n",
-            config_strct->max_write_zeroes_sectors);
-    DBG("uint32_t max_write_zeroes_seg: %u\n",
-            config_strct->max_write_zeroes_seg);
-    DBG("uint8_t write_zeroes_may_unmap: %u\n",
-            config_strct->write_zeroes_may_unmap);
-    DBG("uint8_t unused1[3]: %u\n", config_strct->unused1[0]);
-    DBG("uint8_t unused1[3]: %u\n", config_strct->unused1[1]);
-    DBG("uint8_t unused1[3]: %u\n", config_strct->unused1[2]);
-}
-
 void vhost_user_blk_realize(int queue_num, int queue_size)
 {
     int retries;
@@ -491,10 +483,8 @@ void vhost_user_blk_realize(int queue_num, int queue_size)
 
     global_vdev->vhublk->config_wce = 1;
 
-    /* FIXME: We temporarily hardcoded the vrtqueues number */
     global_vdev->vhublk->num_queues = queue_num;
 
-    /* FIXME: We temporarily hardcoded the vrtqueues size */
     global_vdev->vhublk->queue_size = queue_size;
 
     /* NOTE: global_vdev->vqs == vhublk->virtqs */
@@ -525,9 +515,6 @@ void vhost_user_blk_realize(int queue_num, int queue_size)
     DBG("final global_vdev->host_features: 0x%lx\n",
          global_vdev->host_features);
 
-    print_config((uint8_t *)(&global_vdev->vhublk->blkcfg));
-
-    return;
-
+    print_config_blk((uint8_t *)(&global_vdev->vhublk->blkcfg));
 }
 
diff --git a/vhost_user_input.c b/vhost_user_input.c
index cd5cb4f..e33d22c 100644
--- a/vhost_user_input.c
+++ b/vhost_user_input.c
@@ -92,6 +92,13 @@ static struct vhost_dev *vhost_input_get_vhost(VirtIODevice *vdev)
     return vdev->vhuinput->vhost_dev;
 }
 
+static void print_config_input(uint8_t *config_data)
+{
+    struct virtio_input_config *config_strct =
+        (struct virtio_input_config *)config_data;
+    DBG("print_config_input: Not yet implemented\n");
+}
+
 static void vhost_input_class_init(VirtIODevice *vdev)
 {
     DBG("vhost_input_class_init(...)\n");
@@ -100,13 +107,12 @@ static void vhost_input_class_init(VirtIODevice *vdev)
     /* Comment out the following lines to get the local config */
     vdev->vdev_class->get_config = vhost_input_get_config;
     vdev->vdev_class->set_config = vhost_input_set_config;
-
     vdev->vdev_class->get_vhost = vhost_input_get_vhost;
-
     vdev->vhuinput->vdev_input->input_class->realize = vhost_user_input_realize;
     vdev->vhuinput->vdev_input->input_class->change_active =
                                             vhost_input_change_active;
     vdev->vdev_class->update_mem_table = update_mem_table;
+    vdev->vdev_class->print_config = print_config_input;
 }
 
 
diff --git a/vhost_user_input.h b/vhost_user_input.h
index db5ed33..bbed7f2 100644
--- a/vhost_user_input.h
+++ b/vhost_user_input.h
@@ -54,13 +54,11 @@ typedef struct VirtIOInput {
     QTAILQ_HEAD(, VirtIOInputConfig)  cfg_list;
     VirtQueue                         *evt, *sts;
     char                              *serial;
-
     struct {
         virtio_input_event event;
         VirtQueueElement *elem;
     }                                 *queue;
     uint32_t                          qindex, qsize;
-
     bool                              active;
 
 } VirtIOInput;
diff --git a/vhost_user_loopback.c b/vhost_user_loopback.c
index bf5c8da..d448e55 100644
--- a/vhost_user_loopback.c
+++ b/vhost_user_loopback.c
@@ -352,6 +352,7 @@ int vhost_user_set_features(struct vhost_dev *dev,
     (void) dev;
 
     /* Pass hdev as parameter! */
+    DBG("vhost_user_set_features: 0x%lx\n", features);
     return vhost_user_set_u64(VHOST_USER_SET_FEATURES, features,
                               log_enabled);
 }
diff --git a/virtio_loopback.c b/virtio_loopback.c
index 38b4209..91110c4 100644
--- a/virtio_loopback.c
+++ b/virtio_loopback.c
@@ -60,9 +60,6 @@
 #include <pthread.h>
 #include <limits.h>
 
-/* TODO: Deleteit, only for testing */
-#include <linux/virtio_blk.h>
-
 #ifdef DEBUG
 #define DBG(...) printf("virtio-loopback: " __VA_ARGS__)
 #else
@@ -936,9 +933,9 @@ err:
 void print_neg_flag(uint64_t neg_flag, bool read)
 {
     if (read) {
-        DBG("Read:\n\t");
+        DBG("Read:\n");
     } else {
-        DBG("Write:\n\t");
+        DBG("Write:\n");
     }
 
     switch (neg_flag) {
@@ -1043,8 +1040,6 @@ void print_neg_flag(uint64_t neg_flag, bool read)
         }
         return;
     }
-
-
 }
 
 int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
@@ -1053,7 +1048,7 @@ int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
 
     val &= vdev->host_features;
 
-    vdev->guest_features = val;
+    vdev->guest_features |= val;
     return bad ? -1 : 0;
 }
 
@@ -1385,7 +1380,6 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
     }
 }
 
-
 uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr)
 {
     VirtioDeviceClass *k = vdev->vdev_class;
@@ -1491,8 +1485,6 @@ static uint64_t virtio_loopback_read(VirtIODevice *vdev, uint64_t offset,
 
     uint64_t ret;
 
-    print_neg_flag(offset, 1);
-
     if (!vdev) {
         /*
          * If no backend is present, we treat most registers as
@@ -1522,9 +1514,6 @@ static uint64_t virtio_loopback_read(VirtIODevice *vdev, uint64_t offset,
     if (offset >= VIRTIO_MMIO_CONFIG) {
         offset -= VIRTIO_MMIO_CONFIG;
 
-        /* TODO: To be implemented */
-        DBG("VIRTIO_MMIO_CONFIG: size: %u, offset: %lu\n", size, offset);
-
         if (proxy->legacy) {
             switch (size) {
             case 1:
@@ -1539,7 +1528,7 @@ static uint64_t virtio_loopback_read(VirtIODevice *vdev, uint64_t offset,
             default:
                 abort();
             }
-            DBG("VIRTIO_MMIO_CONFIG: ret: %lu\n", ret);
+            DBG("ret: %lu\n", ret);
             return ret;
         }
 
@@ -1567,13 +1556,11 @@ static uint64_t virtio_loopback_read(VirtIODevice *vdev, uint64_t offset,
     case VIRTIO_MMIO_DEVICE_FEATURES:
         if (proxy->legacy) {
             if (proxy->host_features_sel) {
-                DBG("attempt to read host features with "
-                       "host_features_sel > 0 in legacy mode\n");
-                DBG("vdev->host_features: 0x%lx\n", vdev->host_features);
-                return 0;
+                DBG("vdev->host_features: 0x%lx\n", (vdev->host_features >> 32));
+                return (vdev->host_features >> 32);
             } else {
-                DBG("vdev->host_features: 0x%lx\n", vdev->host_features);
-                return vdev->host_features;
+                DBG("vdev->host_features: 0x%lx\n", vdev->host_features & (uint64_t)(((1ULL << 32) - 1)));
+                return (vdev->host_features & (uint64_t)(((1ULL << 32) - 1)));
             }
         } else {
              /* TODO: To be implemented */
@@ -1648,8 +1635,6 @@ void virtio_loopback_write(VirtIODevice *vdev, uint64_t offset,
                        uint64_t value, unsigned size)
 {
 
-    print_neg_flag(offset, 0);
-
     if (!vdev) {
         /*
          * If no backend is present, we just make all registers
@@ -1662,9 +1647,6 @@ void virtio_loopback_write(VirtIODevice *vdev, uint64_t offset,
     if (offset >= VIRTIO_MMIO_CONFIG) {
         offset -= VIRTIO_MMIO_CONFIG;
 
-        /* TODO: To be implemented */
-        DBG("VIRTIO_MMIO_CONFIG flag write\n");
-
         if (proxy->legacy) {
             switch (size) {
             case 1:
@@ -1682,7 +1664,6 @@ void virtio_loopback_write(VirtIODevice *vdev, uint64_t offset,
             }
             return;
         }
-        DBG("write: VIRTIO_MMIO_CONFIG\n");
 
         return;
     }
@@ -1703,7 +1684,8 @@ void virtio_loopback_write(VirtIODevice *vdev, uint64_t offset,
             if (proxy->guest_features_sel) {
                 DBG("attempt to write guest features with "
                        "guest_features_sel > 0 in legacy mode\n");
-                DBG("Set driver features: 0x%lx\n", value);
+                DBG("Set driver features: 0x%lx\n", value << 32);
+                virtio_set_features(vdev, value << 32);
             } else {
                 DBG("Set driver features: 0x%lx\n", value);
                 virtio_set_features(vdev, value);
@@ -1735,6 +1717,7 @@ void virtio_loopback_write(VirtIODevice *vdev, uint64_t offset,
         }
         break;
     case VIRTIO_MMIO_QUEUE_NUM:
+        DBG("VIRTIO_MMIO_QUEUE_NUM: %lu\n", value);
 
         virtio_queue_set_num(vdev, vdev->queue_sel, value);
 
@@ -1900,6 +1883,7 @@ void adapter_read_write_cb(void)
      *
      * print_neg_flag (address->notification, address->read);
      */
+    print_neg_flag (address->notification, address->read);
 
     if (address->read) {
         address->data = virtio_loopback_read(global_vdev,
diff --git a/virtio_loopback.h b/virtio_loopback.h
index ae0a8b4..25f116f 100644
--- a/virtio_loopback.h
+++ b/virtio_loopback.h
@@ -563,6 +563,7 @@ typedef struct VirtioDeviceClass {
     bool (*primary_unplug_pending)(void *opaque);
 
     void (*update_mem_table)(VirtIODevice *vdev);
+    void (*print_config)(uint8_t *config_data);
 
     struct vhost_dev *(*get_vhost)(VirtIODevice *vdev);
 } VirtioDeviceClass;