Skip to content

Commit 79eea6e

Browse files
virtio-queue: add needs_notification proof for needs(false) condition
This simplifies the logic for the existing 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 79eea6e

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)