[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH v2 03/12] plat/drivers: Init virtio block device
Hi Justin,
Please see inline.
Thanks,
Roxana
On 26.11.2019 08:38, Justin He (Arm Technology China) wrote:
Hi Roxana
-----Original Message-----
From: Minios-devel <minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf
Of Roxana Nicolescu
Sent: Wednesday, October 30, 2019 6:46 PM
To: minios-devel@xxxxxxxxxxxxx
Cc: sharan.santhanam@xxxxxxxxx
Subject: [Minios-devel] [UNIKRAFT PATCH v2 03/12] plat/drivers: Init virtio
block device
This patch introduces the initialization of virtio block device.
First, the driver sets what features supports, and then the driver
negotiates with the device for the common features.
Signed-off-by: Roxana Nicolescu <nicolescu.roxana1996@xxxxxxxxx>
---
plat/drivers/include/virtio/virtio_blk.h | 52 ++++++++++++++
plat/drivers/virtio/virtio_blk.c | 88 ++++++++++++++++++++++++
2 files changed, 140 insertions(+)
create mode 100644 plat/drivers/include/virtio/virtio_blk.h
diff --git a/plat/drivers/include/virtio/virtio_blk.h
b/plat/drivers/include/virtio/virtio_blk.h
new file mode 100644
index 00000000..48625258
--- /dev/null
+++ b/plat/drivers/include/virtio/virtio_blk.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/* This header is BSD licensed so anyone can use the definitions to
implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ * may be used to endorse or promote products derived from this
software
+ * without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS
+ * ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
POSSIBILITY OF
+ * SUCH DAMAGE.
+ **/
+/**
+ * Taken and modified from Linux kernel
+ * include/uapi/linux/virtio_blk.h
+ *
+ * commit-id: 8005803a2
+ */
+#ifndef __PLAT_DRV_VIRTIO_BLK_H
+#define __PLAT_DRV_VIRTIO_BLK_H
+#include <virtio/virtio_ids.h>
+#include <virtio/virtio_config.h>
+#include <virtio/virtio_types.h>
+
+/* Feature bits */
+#define VIRTIO_BLK_F_RO 5 /* Disk is read-only */
+#define VIRTIO_BLK_F_BLK_SIZE 6 /* Block size of disk is
available*/
+
+struct virtio_blk_config {
+ /* The capacity (in 512-byte sectors). */
+ __u64 capacity;
+ /* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
+ __u32 blk_size;
+} __packed;
+
+#endif /* __PLAT_DRV_VIRTIO_BLK_H */
diff --git a/plat/drivers/virtio/virtio_blk.c b/plat/drivers/virtio/virtio_blk.c
index 884f86d9..09724d96 100644
--- a/plat/drivers/virtio/virtio_blk.c
+++ b/plat/drivers/virtio/virtio_blk.c
@@ -28,13 +28,22 @@
#include <virtio/virtio_bus.h>
#include <virtio/virtio_ids.h>
#include <uk/blkdev.h>
+#include <virtio/virtio_blk.h>
#include <uk/blkdev_driver.h>
#define DRIVER_NAME "virtio-blk"
+#define DEFAULT_SECTOR_SIZE 512
#define to_virtioblkdev(bdev) \
__containerof(bdev, struct virtio_blk_device, blkdev)
+/* Features are:
+ * Access Mode
+ * Sector_size;
+ **/
+#define VIRTIO_BLK_DRV_FEATURES(features) \
+ (VIRTIO_FEATURES_UPDATE(features, VIRTIO_BLK_F_RO | \
+ VIRTIO_BLK_F_BLK_SIZE))
static struct uk_alloc *a;
static const char *drv_name = DRIVER_NAME;
@@ -44,6 +53,74 @@ struct virtio_blk_device {
struct uk_blkdev blkdev;
/* The blkdevice identifier */
__u16 uid;
+ /* Virtio Device */
+ struct virtio_dev *vdev;
+};
+
+static int virtio_blkdev_feature_negotiate(struct virtio_blk_device *vbdev)
+{
+ struct uk_blkdev_cap *cap;
+ __u64 host_features = 0;
+ int bytes_to_read;
+ __sector sectors;
+ __sector ssize;
+ int rc = 0;
+
+ UK_ASSERT(vbdev);
+ cap = &vbdev->blkdev.capabilities;
+ host_features = virtio_feature_get(vbdev->vdev);
+
+ /* Get size of device */
+ bytes_to_read = virtio_config_get(vbdev->vdev,
+ __offsetof(struct virtio_blk_config, capacity),
+ §ors,
+ sizeof(sectors),
+ 1);
+ if (bytes_to_read != sizeof(sectors)) {
+ uk_pr_err("Failed to get nb of sectors from device %d\n", rc);
+ rc = -EAGAIN;
+ goto exit;
+ }
+
+ if (!virtio_has_features(host_features, VIRTIO_BLK_F_BLK_SIZE)) {
+ ssize = DEFAULT_SECTOR_SIZE;
+ } else {
+ bytes_to_read = virtio_config_get(vbdev->vdev,
+ __offsetof(struct virtio_blk_config, blk_size),
+ &ssize,
+ sizeof(ssize),
+ 1);
+ if (bytes_to_read != sizeof(ssize)) {
+ uk_pr_err("Failed to get ssize from the device %d\n",
+ rc);
+ rc = -EAGAIN;
+ goto exit;
+ }
+ }
+
+ cap->ssize = ssize;
+ cap->sectors = sectors;
+ cap->ioalign = sizeof(void *);
+ cap->mode = (virtio_has_features(
+ host_features, VIRTIO_BLK_F_RO)) ? O_RDONLY :
O_RDWR;
+
+ /**
+ * Mask out features supported by both driver and device.
+ */
+ vbdev->vdev->features &= host_features;
+ virtio_feature_set(vbdev->vdev, vbdev->vdev->features);
+
+exit:
+ return rc;
+}
+
+static inline void virtio_blkdev_feature_set(struct virtio_blk_device *vbdev)
+{
+ vbdev->vdev->features = 0;
+
Can this initialization be removed since it is initialized in
VIRTIO_BLK_DRV_FEATURES?
I think this initialization should stay because `features` variable is
not actually initialized in VIRTIO_BLK_DRV_FEATURES.
It only sets the features bits, without any initialization before. See
VIRTIO_FEATURES_UPDATE.
--
Cheers,
Justin (Jia He)
+ /* Setting the feature the driver support */
+ VIRTIO_BLK_DRV_FEATURES(vbdev->vdev->features);
+}
};
static int virtio_blk_add_dev(struct virtio_dev *vdev)
@@ -57,6 +134,8 @@ static int virtio_blk_add_dev(struct virtio_dev *vdev)
if (!vbdev)
return -ENOMEM;
+ vbdev->vdev = vdev;
+
rc = uk_blkdev_drv_register(&vbdev->blkdev, a, drv_name);
if (rc < 0) {
uk_pr_err("Failed to register virtio_blk device: %d\n", rc);
@@ -64,10 +143,19 @@ static int virtio_blk_add_dev(struct virtio_dev *vdev)
}
vbdev->uid = rc;
+ virtio_blkdev_feature_set(vbdev);
+ rc = virtio_blkdev_feature_negotiate(vbdev);
+ if (rc) {
+ uk_pr_err("Failed to negotiate the device feature %d\n", rc);
+ goto err_negotiate_feature;
+ }
+
uk_pr_info("Virtio-blk device registered with libukblkdev\n");
out:
return rc;
+err_negotiate_feature:
+ virtio_dev_status_update(vbdev->vdev,
VIRTIO_CONFIG_STATUS_FAIL);
err_out:
uk_free(a, vbdev);
goto out;
--
2.17.1
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
