Skip to content

Commit 4a8b048

Browse files
virtio-queue: handle more cases for needs_notification proof
This simplifies the logic for the existing need_notification proof and makes it more general accounting for both when notification is needed and when it is not. Signed-off-by: Siddharth Priya <[email protected]>
1 parent 57ad18a commit 4a8b048

File tree

1 file changed

+31
-31
lines changed

1 file changed

+31
-31
lines changed

virtio-queue/src/queue/verification.rs

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,17 @@ impl kani::Arbitrary for ProofContext {
459459
}
460460
}
461461

462+
/// Helper function to determine if `point` is in the wrapping range `[start, stop)`.
463+
fn pred_in_wrapping_range(start: Wrapping<u16>, stop: Wrapping<u16>, point: Wrapping<u16>) -> bool {
464+
// Check if point is in the range [start, stop) in a wrapping manner.
465+
if start <= stop {
466+
start <= point && point < stop
467+
} else {
468+
// If start > stop, we wrap around the maximum value of u16.
469+
point >= start || point < stop
470+
}
471+
}
472+
462473
/// # Specification (VirtIO 1.3, Section 2.7.7.2: "Device Requirements: Used Buffer Notification Suppression")
463474
///
464475
/// Section 2.7.7.2 deals with device-to-driver notification suppression. It
@@ -468,46 +479,35 @@ impl kani::Arbitrary for ProofContext {
468479
/// specific number of descriptors has been processed. This is done by the
469480
/// driver defining a "used_event" index, which tells the device "please do not
470481
/// notify me until used.ring[used_event] has been written to by you".
482+
///
483+
/// This proof checks:
484+
// - If event_idx is not enabled, a notification is always needed.
485+
// - If event_idx is enabled, a notification is needed if the used_event index
486+
// is in the wrapping range [next_used - num_added, next_used).
471487
#[kani::proof]
472488
// There are no loops anywhere, but kani really enjoys getting stuck in
473489
// std::ptr::drop_in_place. This is a compiler intrinsic that has a "dummy"
474490
// implementation in stdlib that just recursively calls itself. Kani will
475491
// generally unwind this recursion infinitely.
476492
#[kani::unwind(0)]
477-
fn verify_device_notification_suppression() {
478-
let ProofContext {
479-
mut queue,
480-
memory: mem,
481-
} = kani::any();
482-
483-
let num_added_old = queue.num_added.0;
484-
let needs_notification = queue.needs_notification(&mem);
493+
fn verify_device_to_driver_notification() {
494+
let ProofContext { mut queue, memory } = kani::any();
485495

486-
// event_idx_enabled equivalent to VIRTIO_F_EVENT_IDX negotiated
496+
// If the event_idx_enabled is false, we always need a notification
487497
if !queue.event_idx_enabled {
488-
// The specification here says
489-
// After the device writes a descriptor index into the used ring:
490-
// – If flags is 1, the device SHOULD NOT send a notification.
491-
// – If flags is 0, the device MUST send a notification
492-
// flags is the first field in the avail_ring_address, which we completely ignore. We
493-
// always send a notification, and as there only is a SHOULD NOT, that is okay
494-
assert!(needs_notification.unwrap());
498+
assert!(queue.needs_notification(&memory).unwrap());
495499
} else {
496-
// next_used - 1 is where the previous descriptor was placed
497-
if Wrapping(queue.used_event(&mem, Ordering::Relaxed).unwrap())
498-
== std::num::Wrapping(queue.next_used - Wrapping(1))
499-
&& num_added_old > 0
500-
{
501-
// If the idx field in the used ring (which determined where that descriptor index
502-
// was placed) was equal to used_event, the device MUST send a
503-
// notification.
504-
assert!(needs_notification.unwrap());
505-
506-
kani::cover!();
507-
}
508-
509-
// The other case is handled by a "SHOULD NOT send a notification" in the spec.
510-
// So we do not care
500+
// If the event_idx_enabled is true, we only need a notification if
501+
// the used_event is in the range of [next_used - num_added, next_used)
502+
let used_event = queue.used_event(&memory, Ordering::Relaxed).unwrap();
503+
let current_used = queue.next_used;
504+
let old = current_used - queue.num_added;
505+
let should_notify = pred_in_wrapping_range(old, current_used, used_event);
506+
assert_eq!(
507+
queue.needs_notification(&memory).unwrap(), /* actual */
508+
should_notify, /* expected */
509+
);
510+
assert_eq!(queue.num_added.0, 0);
511511
}
512512
}
513513

0 commit comments

Comments
 (0)