Support readonly feature of VirtIO block device

Add support for comma-separated options for VirtIO devices. For the
readonly option, if the string readonly is specified, subsequent VirtIO
block device access will be read-only; otherwise, the default behavior
is read and write. To implement this, a device_features field is
introduced to store device features during initialization.
VIRTIO_BLK_F_RO is the feature bit to enable readonly.

This feature enhances the security of the VirtIO block device by
preventing unintended data corruption in the guest OS when accessing
critical data.

Additionally, after implementing the VirtIO block device in sysprog21#539, some
register access errors were shown and they are fixed in this commit:
- reading from the VirtIO DeviceFeatures register now correctly checks
  the device_features_sel instead of driver_features_sel.
- writing to the VirtIO DeviceFeaturesSel register now correctly updates
  device_features_sel instead of driver_features_sel.

Author: ChinYikMing

src/devices/virtio-blk.c

@@ -17,6 +17,7 @@
 
 #define DISK_BLK_SIZE 512
 
+
 /* TODO: Enable mutiple virtio-blk devices. */
 #define VBLK_DEV_CNT_MAX 1
 
@@ -94,11 +95,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 +190,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 +287,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 +313,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 +379,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 +401,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 +415,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 +432,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,25 @@ 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];
+        memset(vblk_opts, 0, sizeof(char *[MAX_OPTS]));
+        int vblk_opt_idx = 0;
+        char *opt = strtok(attr->data.system.vblk_device, ",");
+        while (opt) {
+            vblk_opts[vblk_opt_idx++] = opt;
+            opt = strtok(NULL, ",");
+        }
+        char *vblk_device = vblk_opts[0];
+        char *vblk_readonly = vblk_opts[1];
+
+        attr->vblk = vblk_new();
+        attr->vblk->ram = (uint32_t *) attr->mem->mem_base;
+        attr->disk = virtio_blk_init(attr->vblk, vblk_device,
+                                     vblk_readonly ? true : false);
+    }
 
     capture_keyboard_input();
 #endif /* !RV32_HAS(SYSTEM) || (RV32_HAS(SYSTEM) && RV32_HAS(ELF_LOADER)) */