-
Notifications
You must be signed in to change notification settings - Fork 156
drivers: add ramdisk storage driver
#2096
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
base: master
Are you sure you want to change the base?
Conversation
| return irangel(e->base, e->length); | ||
| } | ||
| return irange(INVALID_PHYSICAL, INVALID_PHYSICAL); | ||
| } |
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 having this function defined in all architectures, it's better to pass the ramdisk range directly to init_ramdisk() as a function argument, and call init_ramdisk() only when there is an actual ramdisk.
| token = runtime_strtok_r(&input, delim, &rest); | ||
| while (!sstring_is_null(token)) { | ||
| if(runtime_strcmp(token, ss("verbose")) == 0) { | ||
| cmdline_verbose_logging = true; |
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.
I would remove the cmdline_verbose_logging variable, and keep logging confined to when INIT_DEBUG is #defined; after all, these logging messages are useful mostly when debugging, and I would rather avoid carrying this debug stuff in production kernels.
| storage, st, boolean, write, | ||
| void *buf, range blocks, status_handler sh); | ||
|
|
||
| typedef struct storage |
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.
Better to avoid such a generic name, I would add a ramdisk-related prefix, e.g. something like struct ramdisk_storage
| { | ||
| closure_struct(storage_simple_req_handler, req_handler); | ||
| closure_struct(ramdisk_io, read); | ||
| closure_struct(ramdisk_io, write); |
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.
It's not necessary to define the ramdisk_io closure struct, you can use closure_struct(block_io) instead, for both read and write. Then, you can have 2 separate functions (defined with closure_func_basic) for read and write: for reading, you would retrieve the ramdisk_storage struct pointer via struct_from_closure(); for writing, you don't need any pointer and just apply the status handler with the error status.
| u64 end_byte_offset = blocks.end * SECTOR_SIZE; | ||
| if (start_byte_offset > end_byte_offset || end_byte_offset > st->ramdisk_size) | ||
| { | ||
| apply(sh, timm("result", "read out of bounds")); |
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.
Can use range_valid(blocks); also, instead of applying an error status to the status handler, I would just use assert(), since an out-of-bounds range would indicate a kernel bug bad enough to warrant taking down the VM.
| return; | ||
| } | ||
| u64 ramdisk_size = range_span(ramdisk_phys); | ||
| u64 v = allocate_u64((heap)heap_virtual_huge(kh), ramdisk_size); |
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.
assert(v != INVALID_PHYSICAL);
| map(v, ramdisk_phys.start, ramdisk_size, pageflags_memory()); | ||
|
|
||
| heap h = heap_locked(kh); | ||
| storage st = allocate(h, sizeof(struct storage)); |
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.
assert(st != INVALID_ADDRESS);
| init_closure(&st->read, ramdisk_io, st, false), | ||
| init_closure(&st->write, ramdisk_io, st, true)), | ||
| ramdisk_size, -1); | ||
| msg_print("RAMDISK: %u bytes at %p", st->ramdisk_size, st->ramdisk); |
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.
Better to use msg_info(). For most things the kernel is quiet during boot, unless explicitly told to be more verbose. (I know, right now LOG_INFO messages cannot be enabled this early during boot, but hopefully we will add this capability in the future.)
| .end: | ||
|
|
||
| _start: | ||
| xor edx, edx |
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.
Should be xor rdx, rdx, so that all 64 bits are zeroed.
This patch adds support for using the kernel ramdisk as read-only block storage.