-
Notifications
You must be signed in to change notification settings - Fork 42
dbs-legacy-devices: arm add rtc device #157
Conversation
please do rust fmt |
done |
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 we use this directly? https://github.com/rust-vmm/vm-superio/blob/main/crates/vm-superio/src/rtc_pl031.rs
This seems to work, I'll try it |
crates/dbs-legacy-devices/Cargo.toml
Outdated
@@ -16,6 +16,7 @@ log = "0.4.14" | |||
serde = { version = "1.0.27", features = ["derive", "rc"] } | |||
vm-superio = "0.5.0" | |||
vmm-sys-util = "0.9.0" | |||
byteorder = "1.4.3" |
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.
Only introduce the dependency for aarch64?
[target.'cfg(target_arch = "aarch64")'.dependencies]
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.
Done, I have remove byteorder.
} | ||
}; | ||
} | ||
if read_ok && data.len() <= 4 { |
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.
Why is it "<= 4" instead of "== 4"?
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.
This should be "== 4" and we will use the vm_superio crate to refactor this file.
964f9a2
to
f0a5ea2
Compare
Done, I have use the vm_superio crate to refactor this file |
Arm export fdt address for building vcpu information. Signed-off-by: jingshan <jingshan@linux.alibaba.com>
arm adds rtc device to provide guest kernel clock related information. Signed-off-by: jingshan <jingshan@linux.alibaba.com>
f0a5ea2
to
7115a2e
Compare
Codecov Report
@@ Coverage Diff @@
## main #157 +/- ##
=======================================
Coverage 85.91% 85.91%
=======================================
Files 79 79
Lines 20254 20254
=======================================
Hits 17401 17401
Misses 2853 2853
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
fn read(&mut self, _base: IoAddress, offset: IoAddress, data: &mut [u8]) { | ||
if data.len() == 4 { | ||
self.rtc | ||
.read(offset.raw_value() as u16, data.try_into().unwrap()) |
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.
Why using try_into().unwrap()
here?
How about directly passing data
?
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.
This was my mistake, I answered the question but forgot to submit it.
Since the input parameters of the read() and write() functions of rtc in vm-superio crate are &[u8; 4], we need to use try_into() to convert &[u8] to &[u8; 4].
} | ||
} | ||
|
||
fn write(&mut self, _base: IoAddress, offset: IoAddress, data: &[u8]) { |
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.
Why read
use &mut [u8]
(which I don't see the usage in read
function) and write
use &[u8]
?
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 read to the buffer and write from the buffer.
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.
Got that
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.
Rebase it again?
LGTM others.
This modification mainly involves two aspects: