-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable VirtIO block to access hostOS /dev/ block devices #572
Conversation
84a967d
to
25ea2c5
Compare
da0053e
to
567e8d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Benchmarks
Benchmark suite | Current: e2e87ee | Previous: d1743ff | Ratio |
---|---|---|---|
Dhrystone |
1318 Average DMIPS over 10 runs |
1315 Average DMIPS over 10 runs |
1.00 |
Coremark |
916.239 Average iterations/sec over 10 runs |
913.984 Average iterations/sec over 10 runs |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
b74d233
to
7a4a903
Compare
9409aa1
to
5163318
Compare
On Apple platform, if |
d67e6e0
to
5db091a
Compare
e3aab19
to
119d07a
Compare
37cf3f3 refines the Linux boot script to validate all new system emulation features, excluding SDL. |
68fb6cd
to
37cf3f3
Compare
.github/workflows/main.yml
Outdated
@@ -265,15 +265,15 @@ jobs: | |||
CC: ${{ steps.install_cc.outputs.cc }} | |||
run: | | |||
make distclean && make INITRD_SIZE=32 ENABLE_SYSTEM=1 $PARALLEL && make ENABLE_SYSTEM=1 artifact $PARALLEL | |||
.ci/boot-linux.sh | |||
sudo .ci/boot-linux.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of running with superuser, can you relax the access permission of loop device and then run the emulator in non-root user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of running with superuser, can you relax the access permission of loop device and then run the emulator in non-root user?
The system validation flow in CI is abstracted into a variable called BOOT_LINUX_TEST which uses sudo only for data preparation and does not use sudo to run .ci/boot-linux.sh, enhancing security when launching the system VM.
1b8c300
to
d1743ff
Compare
The user may not always have a disk image but might have a /dev/x block device, such as a USB drive that they want to share with the guest OS. So, allowing this type of virtio-blk source is intuitive. To support this, ioctl is used to retrieve the actual size of the /dev/x block device. This implementation supports both Apple and Linux platforms. On Apple platforms, mmap() on block devices appears to be unsupported with various flag combinations. To address this, a fallback mechanism is added and used when mmap() fails, using malloc() along with pread(), pwrite() and fsync() on the block device fd to emulate the behavior of mmap(). Additionally, the initial fallback was incomplete, as it only allocated heap memory without loading the block device's content into memory. This commit resolves the issue by properly reading the device contents into the allocated buffer. Since there may be asynchronous exits, a new rv_fsync_device() function is introduced to ensure the block device is properly synchronized during such exits. To fully support this fallback, disk_fd and disk_size are now stored in the vblk state during its initialization. Close sysprog21#544
The Linux boot script previously tested both booting and VirtIO block access simultaneously. This commit refines the boot script to test each guest Linux feature independently. In addition, a new color (yellow) is introduced to clearly indicate which test is currently running in the CI, improving debugging capabilities. For future VirtIO device tests or other new features, TEST_OPTIONS and EXPECT_CMDS can be easily updated to extend the tests, enhancing the overall flexibility of the script. The VirtIO block device image or loop device is prepared by the .ci/boot-linux-prepare.sh script which decouples data and control for system emulation validation in .ci/boot-linux.sh. This script can be extended in the future to support additional VirtIO devices by preparing their associated data prior to Linux boot validation. The system validation flow is abstracted into a variable called BOOT_LINUX_TEST which uses sudo only for data preparation and does not use sudo to run .ci/boot-linux.sh, enhancing security when launching the system VM.
d1743ff
to
e2e87ee
Compare
Thank @ChinYikMing for contributing! |
The user may not always have a disk image but might have a /dev/x block device, such as a USB drive that they want to share with the guest OS. So, allowing this type of virtio-blk source is intuitive. To support this, ioctl is used to retrieve the actual size of the /dev/x block device. This implementation supports both Apple and Linux platforms.
Testing
macOS:
Linux
Might see:
macOS
Linux
Known Issue:
On macOS/arm64, OS v15.3.1, the /dev/ block device cannot be
mmap
with error log:ERROR src/devices/virtio-blk.c:475: Could not map disk <block-device>
.After some debugging, the errno is set to
EINVAL
. Then, reading the man page ofmmap
in the system:The page size in my system is 16KiB, which the block device size aligned of (128MiB % 16KiB == 0). The rest of the requirements appear to be met, so this seems a bit strange to me. If anyone has a macOS/arm64 machine, maybe can give it a try.
Summary by Bito
This pull request significantly enhances the virtio-blk driver, enabling guest OS access to hostOS /dev/ block devices like USB drives. It implements ioctl calls for accurate device size retrieval, improves compatibility for Apple and Linux, and includes better error handling, logging, and updated documentation. Unit tests have been added to ensure reliability.Unit tests added: True
Estimated effort to review (1-5, lower is better): 2