Merge pull request #584 from ChinYikMing/virtio-blk-readonly

Support readonly feature of VirtIO block device

Author: jserv

README.md

@@ -75,10 +75,10 @@ If `ENABLE_ARCH_TEST=1` was previously set, run `make distclean` before proceedi
 $ make ENABLE_SYSTEM=1 system
 ```
 
-Build using run using specified images:
+Build and run using specified images (`readonly` option makes the virtual block device read-only):
 ```shell
 $ make ENABLE_SYSTEM=1
-$ build/rv32emu -k <kernel_img_path> -i <rootfs_img_path> [-x vblk:<virtio_blk_img_path>]
+$ build/rv32emu -k <kernel_img_path> -i <rootfs_img_path> [-x vblk:<virtio_blk_img_path>[,readonly]]
 ```
 
 Build with a larger INITRD_SIZE (e.g., 64 MiB) to run SDL-oriented application because the default 8 MiB is insufficient for SDL-oriented application artifacts:

src/devices/virtio-blk.c

@@ -94,11 +94,13 @@ static void virtio_blk_update_status(virtio_blk_state_t *vblk, uint32_t status)
         return;
 
     /* Reset */
+    uint32_t device_features = vblk->device_features;
     uint32_t *ram = vblk->ram;
     uint32_t *disk = vblk->disk;
     void *priv = vblk->priv;
     uint32_t capacity = VBLK_PRIV(vblk)->capacity;
     memset(vblk, 0, sizeof(*vblk));
+    vblk->device_features = device_features;
     vblk->ram = ram;
     vblk->disk = disk;
     vblk->priv = priv;
@@ -187,6 +189,11 @@ static int virtio_blk_desc_handler(virtio_blk_state_t *vblk,
         virtio_blk_read_handler(vblk, sector, vq_desc[1].addr, vq_desc[1].len);
         break;
     case VIRTIO_BLK_T_OUT:
+        if (vblk->device_features & VIRTIO_BLK_F_RO) { /* readonly */
+            rv_log_error("Fail to write on a read only block device");
+            *status = VIRTIO_BLK_S_IOERR;
+            return -1;
+        }
         virtio_blk_write_handler(vblk, sector, vq_desc[1].addr, vq_desc[1].len);
         break;
     default:
@@ -279,9 +286,9 @@ uint32_t virtio_blk_read(virtio_blk_state_t *vblk, uint32_t addr)
     case _(VendorID):
         return VIRTIO_VENDOR_ID;
     case _(DeviceFeatures):
-        return vblk->driver_features_sel == 0
-                   ? VBLK_FEATURES_0
-                   : (vblk->driver_features_sel == 1 ? VBLK_FEATURES_1 : 0);
+        return vblk->device_features_sel == 0
+                   ? VBLK_FEATURES_0 | vblk->device_features
+                   : (vblk->device_features_sel == 1 ? VBLK_FEATURES_1 : 0);
     case _(QueueNumMax):
         return VBLK_QUEUE_NUM_MAX;
     case _(QueueReady):
@@ -305,7 +312,7 @@ void virtio_blk_write(virtio_blk_state_t *vblk, uint32_t addr, uint32_t value)
 #define _(reg) VIRTIO_##reg
     switch (addr) {
     case _(DeviceFeaturesSel):
-        vblk->driver_features_sel = value;
+        vblk->device_features_sel = value;
         break;
     case _(DriverFeatures):
         vblk->driver_features_sel == 0 ? (vblk->driver_features = value) : 0;
@@ -371,7 +378,9 @@ void virtio_blk_write(virtio_blk_state_t *vblk, uint32_t addr, uint32_t value)
 #undef _
 }
 
-uint32_t *virtio_blk_init(virtio_blk_state_t *vblk, char *disk_file)
+uint32_t *virtio_blk_init(virtio_blk_state_t *vblk,
+                          char *disk_file,
+                          bool readonly)
 {
     if (vblk_dev_cnt >= VBLK_DEV_CNT_MAX) {
         rv_log_error(
@@ -391,7 +400,7 @@ uint32_t *virtio_blk_init(virtio_blk_state_t *vblk, char *disk_file)
     }
 
     /* Open disk file */
-    int disk_fd = open(disk_file, O_RDWR);
+    int disk_fd = open(disk_file, readonly ? O_RDONLY : O_RDWR);
     if (disk_fd < 0) {
         rv_log_error("Could not open %s", disk_file);
         exit(EXIT_FAILURE);
@@ -405,8 +414,9 @@ uint32_t *virtio_blk_init(virtio_blk_state_t *vblk, char *disk_file)
     /* Set up the disk memory */
     uint32_t *disk_mem;
 #if HAVE_MMAP
-    disk_mem = mmap(NULL, VBLK_PRIV(vblk)->disk_size, PROT_READ | PROT_WRITE,
-                    MAP_SHARED, disk_fd, 0);
+    disk_mem = mmap(NULL, VBLK_PRIV(vblk)->disk_size,
+                    readonly ? PROT_READ : (PROT_READ | PROT_WRITE), MAP_SHARED,
+                    disk_fd, 0);
     if (disk_mem == MAP_FAILED)
         goto err;
 #else
@@ -421,6 +431,9 @@ uint32_t *virtio_blk_init(virtio_blk_state_t *vblk, char *disk_file)
     VBLK_PRIV(vblk)->capacity =
         (VBLK_PRIV(vblk)->disk_size - 1) / DISK_BLK_SIZE + 1;
 
+    if (readonly)
+        vblk->device_features = VIRTIO_BLK_F_RO;
+
     return disk_mem;
 
 err:

src/devices/virtio.h

@@ -33,6 +33,9 @@
 #define VIRTIO_BLK_S_IOERR 1
 #define VIRTIO_BLK_S_UNSUPP 2
 
+/* TODO: support more features */
+#define VIRTIO_BLK_F_RO (1 << 5)
+
 /* VirtIO MMIO registers */
 #define VIRTIO_REG_LIST                  \
     _(MagicValue, 0x000)        /* R */  \
@@ -87,6 +90,7 @@ typedef struct {
 
 typedef struct {
     /* feature negotiation */
+    uint32_t device_features;
     uint32_t device_features_sel;
     uint32_t driver_features;
     uint32_t driver_features_sel;
@@ -107,7 +111,9 @@ uint32_t virtio_blk_read(virtio_blk_state_t *vblk, uint32_t addr);
 
 void virtio_blk_write(virtio_blk_state_t *vblk, uint32_t addr, uint32_t value);
 
-uint32_t *virtio_blk_init(virtio_blk_state_t *vblk, char *disk_file);
+uint32_t *virtio_blk_init(virtio_blk_state_t *vblk,
+                          char *disk_file,
+                          bool readonly);
 
 virtio_blk_state_t *vblk_new();
 

src/main.c

@@ -74,7 +74,8 @@ static void print_usage(const char *filename)
 #if RV32_HAS(SYSTEM) && !RV32_HAS(ELF_LOADER)
         "  -k <image> : use <image> as kernel image\n"
         "  -i <image> : use <image> as rootfs\n"
-        "  -x vblk:<image> : use <image> as virtio-blk disk image\n"
+        "  -x vblk:<image>[,readonly] : use <image> as virtio-blk disk image "
+        "(default read and write)\n"
         "  -b <bootargs> : use customized <bootargs> for the kernel\n"
 #endif
         "  -d [filename]: dump registers as JSON to the "

src/riscv.c

@@ -535,9 +535,36 @@ riscv_t *rv_create(riscv_user_t rv_attr)
     attr->uart->out_fd = attr->fd_stdout;
 
     /* setup virtio-blk */
-    attr->vblk = vblk_new();
-    attr->vblk->ram = (uint32_t *) attr->mem->mem_base;
-    attr->disk = virtio_blk_init(attr->vblk, attr->data.system.vblk_device);
+    if (attr->data.system.vblk_device) {
+/* Currently, only used for block image path and permission */
+#define MAX_OPTS 2
+        char *vblk_opts[MAX_OPTS] = {NULL};
+        int vblk_opt_idx = 0;
+        char *opt = strtok(attr->data.system.vblk_device, ",");
+        while (opt) {
+            if (vblk_opt_idx == MAX_OPTS) {
+                rv_log_error("Too many arguments for vblk");
+                break;
+            }
+            vblk_opts[vblk_opt_idx++] = opt;
+            opt = strtok(NULL, ",");
+        }
+        char *vblk_device = vblk_opts[0];
+        char *vblk_readonly = vblk_opts[1];
+
+        bool readonly = false;
+        if (vblk_readonly) {
+            if (strcmp(vblk_readonly, "readonly") != 0) {
+                rv_log_error("Unknown vblk option: %s", vblk_readonly);
+                exit(EXIT_FAILURE);
+            }
+            readonly = true;
+        }
+
+        attr->vblk = vblk_new();
+        attr->vblk->ram = (uint32_t *) attr->mem->mem_base;
+        attr->disk = virtio_blk_init(attr->vblk, vblk_device, readonly);
+    }
 
     capture_keyboard_input();
 #endif /* !RV32_HAS(SYSTEM) || (RV32_HAS(SYSTEM) && RV32_HAS(ELF_LOADER)) */