pci: Let device could trap pci config read/write

Currently device impliments PciDevice trait, it will return config
register to bus trait at pci cfg r/w, then BusDevice trait on behave
of device to do actual pci config r/w.

But vfio device need to handle the pci config r/w by itself, as
vfio device need to transfer this request to kernel.

For pci config read, this patch delete PciDevice->config_registers(),
and add PciDevice->read_config_register(), then BusDevice->
config_register_read() call PciDevice->read_config_register(), finally
Device could trap the PciConfig Read.

For pci config write, it is similiar with pci config read. But the
common code is moved into PciConfiguration.

BUG=none
TEST=none

Change-Id: Ie6bd3a8c94f523d6fb1ef3d1e97d087bb0407d9f
Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.corp-partner.google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1580457
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>

devices/src/pci/ac97.rs

@@ -186,12 +186,12 @@ impl PciDevice for Ac97Dev {
         Ok(ranges)
     }
 
-    fn config_registers(&self) -> &PciConfiguration {
-        &self.config_regs
+    fn read_config_register(&self, reg_idx: usize) -> u32 {
+        self.config_regs.read_reg(reg_idx)
     }
 
-    fn config_registers_mut(&mut self) -> &mut PciConfiguration {
-        &mut self.config_regs
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        (&mut self.config_regs).write_reg(reg_idx, offset, data)
     }
 
     fn keep_fds(&self) -> Vec<RawFd> {

devices/src/pci/pci_configuration.rs

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+use byteorder::{ByteOrder, LittleEndian};
+
 use std::fmt::{self, Display};
 
 use crate::pci::PciInterruptPin;
@@ -288,22 +290,42 @@ impl PciConfiguration {
         *(self.registers.get(reg_idx).unwrap_or(&0xffff_ffff))
     }
 
-    /// Writes a 32bit register to `reg_idx` in the register map.
-    pub fn write_reg(&mut self, reg_idx: usize, value: u32) {
+    /// Writes data to PciConfiguration.registers.
+    /// `reg_idx` - index into PciConfiguration.registers.
+    /// `offset`  - PciConfiguration.registers is in unit of DWord, offset define byte
+    ///             offset in the DWrod.
+    /// `data`    - The data to write.
+    pub fn write_reg(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        let reg_offset = reg_idx * 4 + offset as usize;
+        match data.len() {
+            1 => self.write_byte(reg_offset, data[0]),
+            2 => self.write_word(reg_offset, LittleEndian::read_u16(data)),
+            4 => self.write_dword(reg_offset, LittleEndian::read_u32(data)),
+            _ => (),
+        }
+    }
+
+    /// Writes a 32bit dword to `offset`. `offset` must be 32bit aligned.
+    fn write_dword(&mut self, offset: usize, value: u32) {
+        if offset % 4 != 0 {
+            warn!("bad PCI config dword write offset {}", offset);
+            return;
+        }
+        let reg_idx = offset / 4;
         if let Some(r) = self.registers.get_mut(reg_idx) {
             *r = (*r & !self.writable_bits[reg_idx]) | (value & self.writable_bits[reg_idx]);
         } else {
-            warn!("bad PCI register write {}", reg_idx);
+            warn!("bad PCI dword write {}", offset);
         }
     }
 
     /// Writes a 16bit word to `offset`. `offset` must be 16bit aligned.
-    pub fn write_word(&mut self, offset: usize, value: u16) {
+    fn write_word(&mut self, offset: usize, value: u16) {
         let shift = match offset % 4 {
             0 => 0,
             2 => 16,
             _ => {
-                warn!("bad PCI config write offset {}", offset);
+                warn!("bad PCI config word write offset {}", offset);
                 return;
             }
         };
@@ -315,12 +337,12 @@ impl PciConfiguration {
             let shifted_value = (u32::from(value) << shift) & writable_mask;
             *r = *r & !mask | shifted_value;
         } else {
-            warn!("bad PCI config write offset {}", offset);
+            warn!("bad PCI config word write offset {}", offset);
         }
     }
 
     /// Writes a byte to `offset`.
-    pub fn write_byte(&mut self, offset: usize, value: u8) {
+    fn write_byte(&mut self, offset: usize, value: u8) {
         self.write_byte_internal(offset, value, true);
     }
 
@@ -339,7 +361,7 @@ impl PciConfiguration {
             let shifted_value = (u32::from(value) << shift) & writable_mask;
             *r = *r & !mask | shifted_value;
         } else {
-            warn!("bad PCI config write offset {}", offset);
+            warn!("bad PCI config byte write offset {}", offset);
         }
     }
 
@@ -641,7 +663,7 @@ mod tests {
         );
 
         // Attempt to overwrite vendor ID and device ID, which are read-only
-        cfg.write_reg(0, 0xBAADF00D);
+        cfg.write_reg(0, 0, &[0xBA, 0xAD, 0xF0, 0x0D]);
         // The original vendor and device ID should remain.
         assert_eq!(cfg.read_reg(0), 0x56781234);
     }

devices/src/pci/pci_device.rs

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-use byteorder::{ByteOrder, LittleEndian};
-
 use std;
 use std::fmt::{self, Display};
 use std::os::unix::io::RawFd;
@@ -12,7 +10,7 @@ use kvm::Datamatch;
 use resources::{Error as SystemAllocatorFaliure, SystemAllocator};
 use sys_util::EventFd;
 
-use crate::pci::pci_configuration::{self, PciConfiguration};
+use crate::pci::pci_configuration;
 use crate::pci::PciInterruptPin;
 use crate::BusDevice;
 
@@ -90,10 +88,17 @@ pub trait PciDevice: Send {
     fn ioeventfds(&self) -> Vec<(&EventFd, u64, Datamatch)> {
         Vec::new()
     }
-    /// Gets the configuration registers of the Pci Device.
-    fn config_registers(&self) -> &PciConfiguration; // TODO - remove these
-    /// Gets the configuration registers of the Pci Device for modification.
-    fn config_registers_mut(&mut self) -> &mut PciConfiguration;
+
+    /// Reads from a PCI configuration register.
+    /// * `reg_idx` - PCI register index (in units of 4 bytes).
+    fn read_config_register(&self, reg_idx: usize) -> u32;
+
+    /// Writes to a PCI configuration register.
+    /// * `reg_idx` - PCI register index (in units of 4 bytes).
+    /// * `offset`  - byte offset within 4-byte register.
+    /// * `data`    - The data to write.
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]);
+
     /// Reads from a BAR region mapped in to the device.
     /// * `addr` - The guest address inside the BAR.
     /// * `data` - Filled with the data from `addr`.
@@ -124,21 +129,11 @@ impl<T: PciDevice> BusDevice for T {
             return;
         }
 
-        let regs = self.config_registers_mut();
-
-        match data.len() {
-            1 => regs.write_byte(reg_idx * 4 + offset as usize, data[0]),
-            2 => regs.write_word(
-                reg_idx * 4 + offset as usize,
-                (data[0] as u16) | (data[1] as u16) << 8,
-            ),
-            4 => regs.write_reg(reg_idx, LittleEndian::read_u32(data)),
-            _ => (),
-        }
+        self.write_config_register(reg_idx, offset, data)
     }
 
     fn config_register_read(&self, reg_idx: usize) -> u32 {
-        self.config_registers().read_reg(reg_idx)
+        self.read_config_register(reg_idx)
     }
 
     fn on_sandboxed(&mut self) {
@@ -178,11 +173,11 @@ impl<T: PciDevice + ?Sized> PciDevice for Box<T> {
     fn ioeventfds(&self) -> Vec<(&EventFd, u64, Datamatch)> {
         (**self).ioeventfds()
     }
-    fn config_registers(&self) -> &PciConfiguration {
-        (**self).config_registers()
+    fn read_config_register(&self, reg_idx: usize) -> u32 {
+        (**self).read_config_register(reg_idx)
     }
-    fn config_registers_mut(&mut self) -> &mut PciConfiguration {
-        (**self).config_registers_mut()
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        (**self).write_config_register(reg_idx, offset, data)
     }
     fn read_bar(&mut self, addr: u64, data: &mut [u8]) {
         (**self).read_bar(addr, data)

devices/src/pci/pci_root.rs

@@ -26,12 +26,12 @@ impl PciDevice for PciRootConfiguration {
     fn keep_fds(&self) -> Vec<RawFd> {
         Vec::new()
     }
-    fn config_registers(&self) -> &PciConfiguration {
-        &self.config
+    fn read_config_register(&self, reg_idx: usize) -> u32 {
+        self.config.read_reg(reg_idx)
     }
 
-    fn config_registers_mut(&mut self) -> &mut PciConfiguration {
-        &mut self.config
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        (&mut self.config).write_reg(reg_idx, offset, data)
     }
 
     fn read_bar(&mut self, _addr: u64, _data: &mut [u8]) {}

devices/src/usb/xhci/xhci_controller.rs

@@ -234,12 +234,12 @@ impl PciDevice for XhciController {
         Ok(vec![(bar0_addr, XHCI_BAR0_SIZE)])
     }
 
-    fn config_registers(&self) -> &PciConfiguration {
-        &self.config_regs
+    fn read_config_register(&self, reg_idx: usize) -> u32 {
+        self.config_regs.read_reg(reg_idx)
     }
 
-    fn config_registers_mut(&mut self) -> &mut PciConfiguration {
-        &mut self.config_regs
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        (&mut self.config_regs).write_reg(reg_idx, offset, data)
     }
 
     fn read_bar(&mut self, addr: u64, data: &mut [u8]) {

devices/src/virtio/virtio_pci_device.rs

@@ -428,12 +428,12 @@ impl PciDevice for VirtioPciDevice {
             .collect()
     }
 
-    fn config_registers(&self) -> &PciConfiguration {
-        &self.config_regs
+    fn read_config_register(&self, reg_idx: usize) -> u32 {
+        self.config_regs.read_reg(reg_idx)
     }
 
-    fn config_registers_mut(&mut self) -> &mut PciConfiguration {
-        &mut self.config_regs
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        (&mut self.config_regs).write_reg(reg_idx, offset, data)
     }
 
     // Clippy: the value of COMMON_CONFIG_BAR_OFFSET happens to be zero so the