fix(virtio-pci): next_ptr + cfg_ptr offsets
#1218
Conversation
jounathaen
force-pushed
the
virtio-bugfix
branch
2 times, most recently
from
8c42956 to
7cd62fd
Compare
src/drivers/virtio/transport/pci.rs
Outdated
| ); | ||
| let notify_off_multiplier = cap | ||
| .device | ||
| .read_register(u16::try_from(cap.origin.cfg_ptr).unwrap() + 16); |
There was a problem hiding this comment.
Why do is 16 correct and cap.origin.cap_struct.cap_len wrong?
Is the following line also wrong?
https://github.com/hermit-os/kernel/blob/main/src/drivers/virtio/transport/pci.rs#L950-L952
There was a problem hiding this comment.
This needs to read the last 4 bytes of the struct. Adding cap_len results in reading the first byte of the next struct. Likely that the other calculation is also incorrect.
There was a problem hiding this comment.
I have fixed the other two uses of cfg_ptr too in the latest commits. Could you take a look?
There was a problem hiding this comment.
A little verbose, but looks good 👍
|
Before merging, can sbdy test if this breaks something with QEMU? I don't have a test setup at hand |
CI passes and we test virtio-net-pci there, so everything should be fine, I think? |
mkroening
changed the title
cfg_ptrcfg_ptr
Bugfix: Use correct pointer in PCI capability, not the one to the proceeding element Bugfix: Virtio Capabilities - use correct offsets from `cfg_ptr` Co-authored-by: Martin Kröning <[email protected]> Signed-off-by: Martin Kröning <[email protected]>
mkroening
changed the title
cfg_ptrnext_ptr + cfg_ptr offsets
These are two Bugs, where I am wondering how Hermit has event worked before... 🤔