From e09e4701776719dd53062418e8432ee30c0bdfa5 Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Tue, 23 Apr 2019 17:14:32 +0800 Subject: [PATCH] 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 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1580457 Reviewed-by: Daniel Verkamp Tested-by: Daniel Verkamp Tested-by: kokoro Commit-Queue: Daniel Verkamp --- devices/src/pci/ac97.rs | 8 ++--- devices/src/pci/pci_configuration.rs | 40 ++++++++++++++++++------ devices/src/pci/pci_device.rs | 41 +++++++++++-------------- devices/src/pci/pci_root.rs | 8 ++--- devices/src/usb/xhci/xhci_controller.rs | 8 ++--- devices/src/virtio/virtio_pci_device.rs | 8 ++--- 6 files changed, 65 insertions(+), 48 deletions(-) diff --git a/devices/src/pci/ac97.rs b/devices/src/pci/ac97.rs index ff6c1e2597..2deef03690 100644 --- a/devices/src/pci/ac97.rs +++ b/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 { diff --git a/devices/src/pci/pci_configuration.rs b/devices/src/pci/pci_configuration.rs index 566d9b8967..4e77f22860 100644 --- a/devices/src/pci/pci_configuration.rs +++ b/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); } diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs index 8ea3548f3f..4c62e0569f 100644 --- a/devices/src/pci/pci_device.rs +++ b/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 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 PciDevice for Box { 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) diff --git a/devices/src/pci/pci_root.rs b/devices/src/pci/pci_root.rs index 205524aad1..390ff32f67 100644 --- a/devices/src/pci/pci_root.rs +++ b/devices/src/pci/pci_root.rs @@ -26,12 +26,12 @@ impl PciDevice for PciRootConfiguration { fn keep_fds(&self) -> Vec { 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]) {} diff --git a/devices/src/usb/xhci/xhci_controller.rs b/devices/src/usb/xhci/xhci_controller.rs index 453f056194..78022d3c6a 100644 --- a/devices/src/usb/xhci/xhci_controller.rs +++ b/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]) { diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs index a9e54fadc0..2e437b6198 100644 --- a/devices/src/virtio/virtio_pci_device.rs +++ b/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