From 18203fa00f4dcfbc5cd3f05d872756d043ffc235 Mon Sep 17 00:00:00 2001
From: Daniel Verkamp <dverkamp@chromium.org>
Date: Mon, 16 May 2022 14:26:31 -0700
Subject: [PATCH] devices: virtio-pci: refactor BAR lookup

The read_bar() and write_bar() functions had an open-coded equivalent of
BAR lookup to find out if the access was intended for the settings BAR
or one of the device-specific extra BARs. Instead, we can reuse the same
BAR lookup code to find the index of the relevant BAR and compare it
against the settings_bar field.

BUG=b:232838930
TEST=tools/presubmit

Change-Id: I2e5f3ccff47d31718e835ccb745db75dbda2e4ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3664281
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Anton Romanov <romanton@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
---
 devices/src/pci/pci_configuration.rs    |  4 ++
 devices/src/virtio/virtio_pci_device.rs | 58 ++++++++++++++-----------
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/devices/src/pci/pci_configuration.rs b/devices/src/pci/pci_configuration.rs
index ad9db6942f..1cd9400061 100644
--- a/devices/src/pci/pci_configuration.rs
+++ b/devices/src/pci/pci_configuration.rs
@@ -707,6 +707,10 @@ impl PciBarConfiguration {
         self.addr
     }
 
+    pub fn address_range(&self) -> std::ops::Range<u64> {
+        self.addr..self.addr + self.size
+    }
+
     pub fn set_address(mut self, addr: u64) -> Self {
         self.addr = addr;
         self
diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs
index 17b5da867b..c7bd836a17 100644
--- a/devices/src/virtio/virtio_pci_device.rs
+++ b/devices/src/virtio/virtio_pci_device.rs
@@ -22,9 +22,10 @@ use vm_memory::GuestMemory;
 
 use super::*;
 use crate::pci::{
-    BarRange, MsixCap, MsixConfig, PciAddress, PciBarConfiguration, PciBarPrefetchable,
-    PciBarRegionType, PciCapability, PciCapabilityID, PciClassCode, PciConfiguration, PciDevice,
-    PciDeviceError, PciDisplaySubclass, PciHeaderType, PciId, PciInterruptPin, PciSubclass,
+    BarRange, MsixCap, MsixConfig, PciAddress, PciBarConfiguration, PciBarIndex,
+    PciBarPrefetchable, PciBarRegionType, PciCapability, PciCapabilityID, PciClassCode,
+    PciConfiguration, PciDevice, PciDeviceError, PciDisplaySubclass, PciHeaderType, PciId,
+    PciInterruptPin, PciSubclass,
 };
 use crate::virtio::ipc_memory_mapper::IpcMemoryMapper;
 use crate::IrqLevelEvent;
@@ -665,18 +666,17 @@ impl PciDevice for VirtioPciDevice {
     }
 
     fn read_bar(&mut self, addr: u64, data: &mut [u8]) {
-        // The driver is only allowed to do aligned, properly sized access.
-        let bar0 = self.config_regs.get_bar_addr(self.settings_bar as usize);
-        if addr < bar0 || addr >= bar0 + CAPABILITY_BAR_SIZE {
-            let bar_config = self.config_regs.get_bars().find(|config| {
-                addr >= config.address() && addr < (config.address() + config.size())
-            });
-            if let Some(c) = bar_config {
-                self.device
-                    .read_bar(c.bar_index(), addr - c.address(), data);
-            }
-        } else {
-            let offset = addr - bar0;
+        let bar = match self
+            .config_regs
+            .get_bars()
+            .find(|bar| bar.address_range().contains(&addr))
+        {
+            Some(bar) => bar,
+            None => return,
+        };
+
+        if bar.bar_index() == self.settings_bar as PciBarIndex {
+            let offset = addr - bar.address();
             match offset {
                 COMMON_CONFIG_BAR_OFFSET..=COMMON_CONFIG_LAST => self.common_config.read(
                     offset - COMMON_CONFIG_BAR_OFFSET,
@@ -709,21 +709,24 @@ impl PciDevice for VirtioPciDevice {
                 }
                 _ => (),
             }
+        } else {
+            self.device
+                .read_bar(bar.bar_index(), addr - bar.address(), data);
         }
     }
 
     fn write_bar(&mut self, addr: u64, data: &[u8]) {
-        let bar0 = self.config_regs.get_bar_addr(self.settings_bar as usize);
-        if addr < bar0 || addr >= bar0 + CAPABILITY_BAR_SIZE {
-            let bar_config = self.config_regs.get_bars().find(|config| {
-                addr >= config.address() && addr < (config.address() + config.size())
-            });
-            if let Some(c) = bar_config {
-                self.device
-                    .write_bar(c.bar_index(), addr - c.address(), data);
-            }
-        } else {
-            let offset = addr - bar0;
+        let bar = match self
+            .config_regs
+            .get_bars()
+            .find(|bar| bar.address_range().contains(&addr))
+        {
+            Some(bar) => bar,
+            None => return,
+        };
+
+        if bar.bar_index() == self.settings_bar as PciBarIndex {
+            let offset = addr - bar.address();
             match offset {
                 COMMON_CONFIG_BAR_OFFSET..=COMMON_CONFIG_LAST => self.common_config.write(
                     offset - COMMON_CONFIG_BAR_OFFSET,
@@ -758,6 +761,9 @@ impl PciDevice for VirtioPciDevice {
                 }
                 _ => (),
             }
+        } else {
+            self.device
+                .write_bar(bar.bar_index(), addr - bar.address(), data);
         }
 
         if !self.device_activated && self.is_driver_ready() {