Skip to content

fix(pci): capabilities are always little-endian#7

Merged
mkroening merged 1 commit intorust-osdev:mainfrom
Gelbpunkt:pci-endianess
Aug 26, 2025
Merged

fix(pci): capabilities are always little-endian#7
mkroening merged 1 commit intorust-osdev:mainfrom
Gelbpunkt:pci-endianess

Conversation

@Gelbpunkt
Copy link
Copy Markdown
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"

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"

Signed-off-by: Jens Reidel <adrian@travitia.xyz>
Copy link
Copy Markdown
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.

Comment thread src/pci.rs

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

let this = unsafe { mem::transmute::<[u32; 4], Self>(data) };
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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