Skip to content

Conversation

Gelbpunkt
Copy link
Contributor

See https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-1240004

"This virtio structure capability uses little-endian format"

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but one little remark.

@@ -136,6 +136,8 @@ impl Cap {
unsafe { access.read(addr.address, addr.offset + 12) },
];

let data: [u32; 4] = data.map(u32::from_le);

let this = unsafe { mem::transmute::<[u32; 4], Self>(data) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let this = unsafe { mem::transmute::<[u32; 4], Self>(data) };

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you remove this? This is also not touched by the PR. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That line is necessary to convert the raw data read to the capabilities structure, it cannot simply be dropped

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, sorry! I accidentally read it as let this = unsafe { mem::transmute::<[u32; 4], _>(data) }; and thought it is a no-op. 🫠 Time for Feierabend :D

Copy link
Member

@mkroening mkroening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :)

@mkroening mkroening added this pull request to the merge queue Aug 26, 2025
Merged via the queue into rust-osdev:main with commit a7d7294 Aug 26, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants