Skip to content

Commit cc1fa35

Browse files
slpandreeaflorescu
authored andcommitted
queue/tests: Fix endianess of memory operations
In the tests, the driver behavior is emulated by writting directly into the memory. According to the specification, the driver must write virtqueue-related data in little-endian, even if it's running on a big-endian machine. Fix those accesses by using u16::to_le() and u16::from_le() where needed. These operations become no-ops on little-endian machines. With this change, the unit tests pass on big-endian machines (tested on s390x). Signed-off-by: Sergio Lopez <[email protected]>
1 parent 9c35b74 commit cc1fa35

File tree

3 files changed

+43
-36
lines changed

3 files changed

+43
-36
lines changed

crates/virtio-queue/src/iterator.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,10 @@ mod tests {
131131
vq.desc_table().store(j, desc);
132132
}
133133

134-
vq.avail().ring().ref_at(0).store(0);
135-
vq.avail().ring().ref_at(1).store(2);
136-
vq.avail().ring().ref_at(2).store(5);
137-
vq.avail().idx().store(3);
134+
vq.avail().ring().ref_at(0).store(u16::to_le(0));
135+
vq.avail().ring().ref_at(1).store(u16::to_le(2));
136+
vq.avail().ring().ref_at(2).store(u16::to_le(5));
137+
vq.avail().idx().store(u16::to_le(3));
138138

139139
let mut i = q.iter().unwrap();
140140

@@ -206,9 +206,9 @@ mod tests {
206206
vq.desc_table().store(j, desc);
207207
}
208208

209-
vq.avail().ring().ref_at(0).store(0);
210-
vq.avail().ring().ref_at(1).store(2);
211-
vq.avail().idx().store(2);
209+
vq.avail().ring().ref_at(0).store(u16::to_le(0));
210+
vq.avail().ring().ref_at(1).store(u16::to_le(2));
211+
vq.avail().idx().store(u16::to_le(2));
212212

213213
let mut i = q.iter().unwrap();
214214

crates/virtio-queue/src/queue.rs

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -266,16 +266,16 @@ mod tests {
266266
let vq = MockSplitQueue::new(m, 16);
267267
let mut q = vq.create_queue(m);
268268

269-
assert_eq!(vq.used().idx().load(), 0);
269+
assert_eq!(u16::from_le(vq.used().idx().load()), 0);
270270

271271
// index too large
272272
assert!(q.add_used(16, 0x1000).is_err());
273-
assert_eq!(vq.used().idx().load(), 0);
273+
assert_eq!(u16::from_le(vq.used().idx().load()), 0);
274274

275275
// should be ok
276276
q.add_used(1, 0x1000).unwrap();
277277
assert_eq!(q.state.next_used, Wrapping(1));
278-
assert_eq!(vq.used().idx().load(), 1);
278+
assert_eq!(u16::from_le(vq.used().idx().load()), 1);
279279

280280
let x = vq.used().ring().ref_at(0).load();
281281
assert_eq!(x.id(), 1);
@@ -334,8 +334,11 @@ mod tests {
334334
assert_eq!(q.needs_notification().unwrap(), true);
335335
}
336336

337-
m.write_obj::<u16>(4, avail_addr.unchecked_add(4 + qsize as u64 * 2))
338-
.unwrap();
337+
m.write_obj::<u16>(
338+
u16::to_le(4),
339+
avail_addr.unchecked_add(4 + qsize as u64 * 2),
340+
)
341+
.unwrap();
339342
q.state.set_event_idx(true);
340343

341344
// Incrementing up to this value causes an `u16` to wrap back to 0.
@@ -383,26 +386,28 @@ mod tests {
383386
assert_eq!(q.state.event_idx_enabled, false);
384387

385388
q.enable_notification().unwrap();
386-
let v = m.read_obj::<u16>(used_addr).unwrap();
389+
let v = m.read_obj::<u16>(used_addr).map(u16::from_le).unwrap();
387390
assert_eq!(v, 0);
388391

389392
q.disable_notification().unwrap();
390-
let v = m.read_obj::<u16>(used_addr).unwrap();
393+
let v = m.read_obj::<u16>(used_addr).map(u16::from_le).unwrap();
391394
assert_eq!(v, VIRTQ_USED_F_NO_NOTIFY);
392395

393396
q.enable_notification().unwrap();
394-
let v = m.read_obj::<u16>(used_addr).unwrap();
397+
let v = m.read_obj::<u16>(used_addr).map(u16::from_le).unwrap();
395398
assert_eq!(v, 0);
396399

397400
q.set_event_idx(true);
398401
let avail_addr = vq.avail_addr();
399-
m.write_obj::<u16>(2, avail_addr.unchecked_add(2)).unwrap();
402+
m.write_obj::<u16>(u16::to_le(2), avail_addr.unchecked_add(2))
403+
.unwrap();
400404

401405
assert_eq!(q.enable_notification().unwrap(), true);
402406
q.state.next_avail = Wrapping(2);
403407
assert_eq!(q.enable_notification().unwrap(), false);
404408

405-
m.write_obj::<u16>(8, avail_addr.unchecked_add(2)).unwrap();
409+
m.write_obj::<u16>(u16::to_le(8), avail_addr.unchecked_add(2))
410+
.unwrap();
406411

407412
assert_eq!(q.enable_notification().unwrap(), true);
408413
q.state.next_avail = Wrapping(8);
@@ -430,13 +435,13 @@ mod tests {
430435
vq.desc_table().store(i, desc);
431436
}
432437

433-
vq.avail().ring().ref_at(0).store(0);
434-
vq.avail().ring().ref_at(1).store(2);
435-
vq.avail().ring().ref_at(2).store(5);
436-
vq.avail().ring().ref_at(3).store(7);
437-
vq.avail().ring().ref_at(4).store(9);
438+
vq.avail().ring().ref_at(0).store(u16::to_le(0));
439+
vq.avail().ring().ref_at(1).store(u16::to_le(2));
440+
vq.avail().ring().ref_at(2).store(u16::to_le(5));
441+
vq.avail().ring().ref_at(3).store(u16::to_le(7));
442+
vq.avail().ring().ref_at(4).store(u16::to_le(9));
438443
// Let the device know it can consume chains with the index < 2.
439-
vq.avail().idx().store(2);
444+
vq.avail().idx().store(u16::to_le(2));
440445
// No descriptor chains are consumed at this point.
441446
assert_eq!(q.next_avail(), 0);
442447

@@ -461,7 +466,7 @@ mod tests {
461466
// The next chain that can be consumed should have index 2.
462467
assert_eq!(q.next_avail(), 2);
463468
// Let the device know it can consume one more chain.
464-
vq.avail().idx().store(3);
469+
vq.avail().idx().store(u16::to_le(3));
465470
i = 0;
466471

467472
loop {
@@ -476,7 +481,7 @@ mod tests {
476481
// ring. Ideally this should be done on a separate thread.
477482
// Because of this update, the loop should be iterated again to consume the new
478483
// available descriptor chains.
479-
vq.avail().idx().store(4);
484+
vq.avail().idx().store(u16::to_le(4));
480485
if !q.enable_notification().unwrap() {
481486
break;
482487
}
@@ -487,7 +492,7 @@ mod tests {
487492

488493
// Set an `idx` that is bigger than the number of entries added in the ring.
489494
// This is an allowed scenario, but the indexes of the chain will have unexpected values.
490-
vq.avail().idx().store(7);
495+
vq.avail().idx().store(u16::to_le(7));
491496
loop {
492497
q.disable_notification().unwrap();
493498

@@ -525,11 +530,11 @@ mod tests {
525530
vq.desc_table().store(i, desc);
526531
}
527532

528-
vq.avail().ring().ref_at(0).store(0);
529-
vq.avail().ring().ref_at(1).store(2);
530-
vq.avail().ring().ref_at(2).store(5);
533+
vq.avail().ring().ref_at(0).store(u16::to_le(0));
534+
vq.avail().ring().ref_at(1).store(u16::to_le(2));
535+
vq.avail().ring().ref_at(2).store(u16::to_le(5));
531536
// Let the device know it can consume chains with the index < 2.
532-
vq.avail().idx().store(3);
537+
vq.avail().idx().store(u16::to_le(3));
533538
// No descriptor chains are consumed at this point.
534539
assert_eq!(q.next_avail(), 0);
535540

@@ -552,7 +557,7 @@ mod tests {
552557

553558
// Decrement `idx` which should be forbidden. We don't enforce this thing, but we should
554559
// test that we don't panic in case the driver decrements it.
555-
vq.avail().idx().store(1);
560+
vq.avail().idx().store(u16::to_le(1));
556561

557562
loop {
558563
q.disable_notification().unwrap();

crates/virtio-queue/src/state_sync.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -267,26 +267,28 @@ mod tests {
267267

268268
assert_eq!(q.lock_state().event_idx_enabled, false);
269269
q.enable_notification(mem).unwrap();
270-
let v = m.read_obj::<u16>(used_addr).unwrap();
270+
let v = m.read_obj::<u16>(used_addr).map(u16::from_le).unwrap();
271271
assert_eq!(v, 0);
272272

273273
q.disable_notification(m.memory()).unwrap();
274-
let v = m.read_obj::<u16>(used_addr).unwrap();
274+
let v = m.read_obj::<u16>(used_addr).map(u16::from_le).unwrap();
275275
assert_eq!(v, VIRTQ_USED_F_NO_NOTIFY);
276276

277277
q.enable_notification(mem).unwrap();
278-
let v = m.read_obj::<u16>(used_addr).unwrap();
278+
let v = m.read_obj::<u16>(used_addr).map(u16::from_le).unwrap();
279279
assert_eq!(v, 0);
280280

281281
q.set_event_idx(true);
282282
let avail_addr = q.lock_state().avail_ring;
283-
m.write_obj::<u16>(2, avail_addr.unchecked_add(2)).unwrap();
283+
m.write_obj::<u16>(u16::to_le(2), avail_addr.unchecked_add(2))
284+
.unwrap();
284285

285286
assert_eq!(q.enable_notification(mem).unwrap(), true);
286287
q.lock_state().next_avail = Wrapping(2);
287288
assert_eq!(q.enable_notification(mem).unwrap(), false);
288289

289-
m.write_obj::<u16>(8, avail_addr.unchecked_add(2)).unwrap();
290+
m.write_obj::<u16>(u16::to_le(8), avail_addr.unchecked_add(2))
291+
.unwrap();
290292

291293
assert_eq!(q.enable_notification(mem).unwrap(), true);
292294
q.lock_state().next_avail = Wrapping(8);

0 commit comments

Comments
 (0)