Skip to content

Commit ff8ca93

Browse files
committed
refactor: do not redefine x86 constant in test_vsock_bof
This test is all about testing what happens when a vsock driver places virtio buffers close to / around the MMIO gap on x86_64 systems. Since this test thus doesn't really serve a purpose on aarch64, we can just cfg(target_arch = "x86_64") it, and reuse the constants from arch::x86_64 instead of redefining them again. fwiw, I have no idea what "bof" means. I'm also confused by this test passing on ARM in the first place, given that some of the comments indicate to me that it should fail if the buffers overlap the mmio gap (which doesn't exist on arm, so nothing should fail?). Signed-off-by: Patrick Roy <[email protected]>
1 parent ba11d47 commit ff8ca93

File tree

2 files changed

+25
-18
lines changed

2 files changed

+25
-18
lines changed

src/vmm/src/arch/x86_64/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ pub enum ConfigurationError {
7373
StartInfoSetup,
7474
}
7575

76-
const FIRST_ADDR_PAST_32BITS: u64 = 1 << 32;
76+
/// First address that cannot be addressed using 32 bit anymore.
77+
pub const FIRST_ADDR_PAST_32BITS: u64 = 1 << 32;
7778

7879
/// Size of MMIO gap at top of 32-bit address space.
7980
pub const MEM_32BIT_GAP_SIZE: u64 = 768 << 20;

src/vmm/src/devices/virtio/vsock/event_handler.rs

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,7 @@ mod tests {
223223

224224
use super::super::*;
225225
use super::*;
226-
use crate::devices::virtio::vsock::packet::VSOCK_PKT_HDR_SIZE;
227226
use crate::devices::virtio::vsock::test_utils::{EventHandlerContext, TestContext};
228-
use crate::test_utils::multi_region_mem;
229-
use crate::vstate::memory::Bytes;
230227

231228
#[test]
232229
fn test_txq_event() {
@@ -427,8 +424,9 @@ mod tests {
427424
// function for testing error cases, so the asserts always expect is_err() to be true. When
428425
// desc_idx = 0 we are altering the header (first descriptor in the chain), and when
429426
// desc_idx = 1 we are altering the packet buffer.
427+
#[cfg(target_arch = "x86_64")]
430428
fn vsock_bof_helper(test_ctx: &mut TestContext, desc_idx: usize, addr: u64, len: u32) {
431-
use crate::vstate::memory::GuestAddress;
429+
use crate::vstate::memory::{Bytes, GuestAddress};
432430

433431
assert!(desc_idx <= 1);
434432

@@ -472,19 +470,22 @@ mod tests {
472470
}
473471

474472
#[test]
473+
#[cfg(target_arch = "x86_64")]
474+
#[allow(clippy::cast_possible_truncation)] /* casting of constants we know fit into u32 */
475475
fn test_vsock_bof() {
476+
use crate::arch::MMIO_MEM_START;
477+
use crate::arch::x86_64::{FIRST_ADDR_PAST_32BITS, MEM_32BIT_GAP_SIZE};
478+
use crate::devices::virtio::vsock::packet::VSOCK_PKT_HDR_SIZE;
479+
use crate::test_utils::multi_region_mem;
476480
use crate::vstate::memory::GuestAddress;
477481

478-
const GAP_SIZE: u32 = 768 << 20;
479-
const FIRST_AFTER_GAP: usize = 1 << 32;
480-
const GAP_START_ADDR: usize = FIRST_AFTER_GAP - GAP_SIZE as usize;
481482
const MIB: usize = 1 << 20;
482483

483484
let mut test_ctx = TestContext::new();
484485
test_ctx.mem = multi_region_mem(&[
485486
(GuestAddress(0), 8 * MIB),
486-
(GuestAddress((GAP_START_ADDR - MIB) as u64), MIB),
487-
(GuestAddress(FIRST_AFTER_GAP as u64), MIB),
487+
(GuestAddress(MMIO_MEM_START - MIB as u64), MIB),
488+
(GuestAddress(FIRST_ADDR_PAST_32BITS), MIB),
488489
]);
489490

490491
// The default configured descriptor chains are valid.
@@ -506,20 +507,25 @@ mod tests {
506507
}
507508

508509
// Let's check what happens when the header descriptor is right before the gap.
509-
vsock_bof_helper(
510-
&mut test_ctx,
511-
0,
512-
GAP_START_ADDR as u64 - 1,
513-
VSOCK_PKT_HDR_SIZE,
514-
);
510+
vsock_bof_helper(&mut test_ctx, 0, MMIO_MEM_START - 1, VSOCK_PKT_HDR_SIZE);
515511

516512
// Let's check what happens when the buffer descriptor crosses into the gap, but does
517513
// not go past its right edge.
518-
vsock_bof_helper(&mut test_ctx, 1, GAP_START_ADDR as u64 - 4, GAP_SIZE + 4);
514+
vsock_bof_helper(
515+
&mut test_ctx,
516+
1,
517+
MMIO_MEM_START - 4,
518+
MEM_32BIT_GAP_SIZE as u32 + 4,
519+
);
519520

520521
// Let's modify the buffer descriptor addr and len such that it crosses over the MMIO gap,
521522
// and check we cannot assemble the VsockPkts.
522-
vsock_bof_helper(&mut test_ctx, 1, GAP_START_ADDR as u64 - 4, GAP_SIZE + 100);
523+
vsock_bof_helper(
524+
&mut test_ctx,
525+
1,
526+
MMIO_MEM_START - 4,
527+
MEM_32BIT_GAP_SIZE as u32 + 100,
528+
);
523529
}
524530

525531
#[test]

0 commit comments

Comments
 (0)