Devices: vfio: Fix the PCIe BAR region mmap issue

Intel SSD 660P Series NVME block device has an overlapping msix
table and msix pba table as the following information shows.

ab:00.0 Non-Volatile memory controller: Intel Corporation SSD 660P Series (rev 03)
Capabilities: [b0] MSI-X: Enable- Count=22 Masked-
                Vector table: BAR=0 offset=00002000
                PBA: BAR=0 offset=00002100

In curruent add_bar_mmap_msix function, these two table would occupy
the same page. So when add_bar_mmap_msix trys to find the msix table
corresponding pages for the second time, it fails to find the page and
lead to the entire mmaps are cleared and return null mmaps. Thus, there
is no real memory region are mmaped and lead to quite low I/O performance
for this pass-through NVME disk.

BUG=None
TEST=Unit test
TEST=pass-through the 660P SSD into guest and run FIO read/write test,
the throught increases to ~500MBps form ~20MBps.

Change-Id: I1571e694b0a1f01a738650b361eaef93554a8c55
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3213315
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This commit is contained in:
Dapeng Mi 2021-11-24 11:17:36 +00:00 committed by Commit Bot
parent e2a3e29ec6
commit 6087a79074

View file

@ -410,7 +410,7 @@ impl VfioMsixAllocator {
}
// Allocates a range of addresses from the managed region with a required location.
// Returns OutOfSpace if requested range is not available (e.g. already allocated).
// Returns a new range of addresses excluding the required range.
fn allocate_at(&mut self, start: u64, size: u64) -> Result<(), PciDeviceError> {
if size == 0 {
return Err(PciDeviceError::MsixAllocatorSizeZero);
@ -418,13 +418,12 @@ impl VfioMsixAllocator {
let end = start
.checked_add(size - 1)
.ok_or(PciDeviceError::MsixAllocatorOutOfSpace)?;
match self
while let Some(slot) = self
.regions
.iter()
.find(|range| range.0 <= start && range.1 >= end)
.find(|range| (start <= range.1 && end >= range.0))
.cloned()
{
Some(slot) => {
self.regions.remove(&slot);
if slot.0 < start {
self.regions.insert((slot.0, start - 1));
@ -432,11 +431,9 @@ impl VfioMsixAllocator {
if slot.1 > end {
self.regions.insert((end + 1, slot.1));
}
}
Ok(())
}
None => Err(PciDeviceError::MsixAllocatorOutOfSpace),
}
}
}
enum DeviceData {
@ -720,9 +717,7 @@ impl VfioPciDevice {
(min(msix_offset + msix_size, mmap_offset + mmap_size) + pgmask) & !pgmask;
if end > begin {
if let Err(e) = to_mmap.allocate_at(begin, end - begin) {
error!("{} add_bar_mmap_msix failed: {}", self.debug_label(), e);
mmaps.clear();
return mmaps;
error!("add_bar_mmap_msix failed: {}", e);
}
}
}
@ -1267,3 +1262,76 @@ impl PciDevice for VfioPciDevice {
self.close();
}
}
#[cfg(test)]
mod tests {
use super::VfioMsixAllocator;
#[test]
fn no_overlap() {
// regions [32, 95]
let mut memory = VfioMsixAllocator::new(32, 64).unwrap();
memory.allocate_at(0, 16).unwrap();
memory.allocate_at(100, 16).unwrap();
let mut iter = memory.regions.iter();
assert_eq!(iter.next(), Some(&(32, 95)));
}
#[test]
fn full_overlap() {
// regions [32, 95]
let mut memory = VfioMsixAllocator::new(32, 64).unwrap();
// regions [32, 47], [64, 95]
memory.allocate_at(48, 16).unwrap();
// regions [64, 95]
memory.allocate_at(32, 16).unwrap();
let mut iter = memory.regions.iter();
assert_eq!(iter.next(), Some(&(64, 95)));
}
#[test]
fn partial_overlap_one() {
// regions [32, 95]
let mut memory = VfioMsixAllocator::new(32, 64).unwrap();
// regions [32, 47], [64, 95]
memory.allocate_at(48, 16).unwrap();
// regions [32, 39], [64, 95]
memory.allocate_at(40, 16).unwrap();
let mut iter = memory.regions.iter();
assert_eq!(iter.next(), Some(&(32, 39)));
assert_eq!(iter.next(), Some(&(64, 95)));
}
#[test]
fn partial_overlap_two() {
// regions [32, 95]
let mut memory = VfioMsixAllocator::new(32, 64).unwrap();
// regions [32, 47], [64, 95]
memory.allocate_at(48, 16).unwrap();
// regions [32, 39], [72, 95]
memory.allocate_at(40, 32).unwrap();
let mut iter = memory.regions.iter();
assert_eq!(iter.next(), Some(&(32, 39)));
assert_eq!(iter.next(), Some(&(72, 95)));
}
#[test]
fn partial_overlap_three() {
// regions [32, 95]
let mut memory = VfioMsixAllocator::new(32, 64).unwrap();
// regions [32, 39], [48, 95]
memory.allocate_at(40, 8).unwrap();
// regions [32, 39], [48, 63], [72, 95]
memory.allocate_at(64, 8).unwrap();
// regions [32, 35], [76, 95]
memory.allocate_at(36, 40).unwrap();
let mut iter = memory.regions.iter();
assert_eq!(iter.next(), Some(&(32, 35)));
assert_eq!(iter.next(), Some(&(76, 95)));
}
}