Skip to content

Commit fe85d8a

Browse files
committed
uefi: Improve cmp impl of PciIoAddress to something logical you'd expect
This sorting now actually makes sense from a logical point of view. This is now also the ordering you get by using typical tools like lspci.
1 parent bc602a5 commit fe85d8a

File tree

2 files changed

+51
-3
lines changed

2 files changed

+51
-3
lines changed

uefi/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
- Added `proto::ata::AtaRequestBuilder::read_pio()`.
55

66
## Changed
7-
7+
- Changed ordering of `proto::pci::PciIoAddress` to (bus -> dev -> fun -> reg -> ext_reg).
88

99
# uefi - v0.36 (2025-10-21)
1010

uefi/src/proto/pci/mod.rs

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,15 @@ impl PartialOrd for PciIoAddress {
109109
}
110110

111111
impl Ord for PciIoAddress {
112-
fn cmp(&self, other: &Self) -> Ordering {
113-
u64::from(*self).cmp(&u64::from(*other))
112+
fn cmp(&self, o: &Self) -> Ordering {
113+
// extract fields because taking references to unaligned fields in packed structs is a nono
114+
let (bus, dev, fun, reg, ext_reg) = (self.bus, self.dev, self.fun, self.reg, self.ext_reg);
115+
let (o_bus, o_dev, o_fun, o_reg, o_ext_reg) = (o.bus, o.dev, o.fun, o.reg, o.ext_reg);
116+
bus.cmp(&o_bus)
117+
.then(dev.cmp(&o_dev))
118+
.then(fun.cmp(&o_fun))
119+
.then(reg.cmp(&o_reg))
120+
.then(ext_reg.cmp(&o_ext_reg))
114121
}
115122
}
116123

@@ -152,6 +159,8 @@ fn encode_io_mode_and_unit<U: PciIoUnit>(mode: PciIoMode) -> PciRootBridgeIoProt
152159

153160
#[cfg(test)]
154161
mod tests {
162+
use core::cmp::Ordering;
163+
155164
use super::PciIoAddress;
156165

157166
#[test]
@@ -170,4 +179,43 @@ mod tests {
170179
assert_eq!(rawaddr, 0x99_bb_dd_ff_7755_3311);
171180
assert_eq!(srcaddr, dstaddr);
172181
}
182+
183+
#[test]
184+
fn test_pci_order() {
185+
let addr0_0_0 = PciIoAddress::new(0, 0, 0);
186+
let addr0_0_1 = PciIoAddress::new(0, 0, 1);
187+
let addr0_1_0 = PciIoAddress::new(0, 1, 0);
188+
let addr1_0_0 = PciIoAddress::new(1, 0, 0);
189+
190+
assert_eq!(addr0_0_0.cmp(&addr0_0_0), Ordering::Equal);
191+
assert_eq!(addr0_0_0.cmp(addr0_0_1), Ordering::Less);
192+
assert_eq!(addr0_0_0.cmp(&addr0_1_0), Ordering::Less);
193+
assert_eq!(addr0_0_0.cmp(&addr1_0_0), Ordering::Less);
194+
195+
assert_eq!(addr0_0_1.cmp(addr0_0_0), Ordering::Greater);
196+
assert_eq!(addr0_0_1.cmp(addr0_0_1), Ordering::Equal);
197+
assert_eq!(addr0_0_1.cmp(&addr0_1_0), Ordering::Less);
198+
assert_eq!(addr0_0_1.cmp(&addr1_0_0), Ordering::Less);
199+
200+
assert_eq!(addr0_1_0.cmp(addr0_0_0), Ordering::Greater);
201+
assert_eq!(addr0_1_0.cmp(addr0_0_1), Ordering::Greater);
202+
assert_eq!(addr0_1_0.cmp(&addr0_1_0), Ordering::Equal);
203+
assert_eq!(addr0_1_0.cmp(&addr1_0_0), Ordering::Less);
204+
205+
assert_eq!(addr1_0_0.cmp(addr0_0_0), Ordering::Greater);
206+
assert_eq!(addr1_0_0.cmp(addr0_0_1), Ordering::Greater);
207+
assert_eq!(addr1_0_0.cmp(&addr0_1_0), Ordering::Greater);
208+
assert_eq!(addr1_0_0.cmp(&addr1_0_0), Ordering::Equal);
209+
210+
assert_eq!(addr0_0_0.cmp(addr0_0_0.with_register(1)), Ordering::Less);
211+
assert_eq!(addr0_0_0.with_register(1).cmp(addr0_0_0), Ordering::Greater);
212+
assert_eq!(
213+
addr0_0_0.cmp(addr0_0_0.with_extended_register(1)),
214+
Ordering::Less
215+
);
216+
assert_eq!(
217+
addr0_0_0.with_extended_register(1).cmp(addr0_0_0),
218+
Ordering::Greater
219+
);
220+
}
173221
}

0 commit comments

Comments
 (0)