Clean up `create_fdt` function in `src/vmm/src/arch/aarch64/fdt.rs`
roypat opened this issue · 7 comments
The create_fdt function has two eccentricities that can be cleaned up:
- It is unnecessarily parametrized by
std::hash::HashBuilder, and - It both writes the flattened device tree to guest memory, as well as returns it (however this return value is ignored the only call site). In the spirit of separating concerns, it should not write the FDT to guest memory - instead the caller should use the return value and write it to memory. This might also simplify our unittesting a little bit, as we no longer need to construct
GuestMemoryMmapin the tests.
See also #4687 (comment)
Hi @jackabald,
I've assigned you, thanks for having a look!
@roypat, I have made changes to suit your needs regarding this issue. However, I am having trouble configuring the test cases in fdt.rs. I don't want to open a pull request without fully testing this code, but I am having trouble running these test cases.
Should I just send a pull request for you to take a look at?
@roypat, I have made changes to suit your needs regarding this issue. However, I am having trouble configuring the test cases in fdt.rs. I don't want to open a pull request without fully testing this code, but I am having trouble running these test cases.
Should I just send a pull request for you to take a look at?
Yes, opening a draft PR for early feedback is always welcome :)
@roypat linked the pull request. please let me know if I am going in the right direction. I had created a whole new function for writing to guest memory but it seems it's not needed if we just want callers to assign it to memory based on its return value.
Please review my new pull request. If you would like me to work on cleaning up some of the unit testing I will happily work on this further :)
Thank you!