From 8ec87d6d334126f0a3c235e19470f707dc5f02e4 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 24 Oct 2019 16:27:12 -0700 Subject: [PATCH] devices: pci: make get_bar_addr work for all BAR types Previously, PciConfiguration::get_bar_addr would only correctly return the value of a 32-bit memory region; implement support for the other valid BAR types as well. BUG=None TEST=cargo test -p devices Change-Id: I221187dfb96b31d7fead73eccf605a0886021d8b Signed-off-by: Daniel Verkamp Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1880164 Tested-by: kokoro Reviewed-by: Dylan Reid --- devices/src/pci/ac97.rs | 8 +- devices/src/pci/pci_configuration.rs | 123 +++++++++++++++++++++++- devices/src/virtio/virtio_pci_device.rs | 6 +- 3 files changed, 127 insertions(+), 10 deletions(-) diff --git a/devices/src/pci/ac97.rs b/devices/src/pci/ac97.rs index 9a3c7e542d..eb19b5f742 100644 --- a/devices/src/pci/ac97.rs +++ b/devices/src/pci/ac97.rs @@ -203,8 +203,8 @@ impl PciDevice for Ac97Dev { } fn read_bar(&mut self, addr: u64, data: &mut [u8]) { - let bar0 = u64::from(self.config_regs.get_bar_addr(0)); - let bar1 = u64::from(self.config_regs.get_bar_addr(1)); + let bar0 = self.config_regs.get_bar_addr(0); + let bar1 = self.config_regs.get_bar_addr(1); match addr { a if a >= bar0 && a < bar0 + MIXER_REGS_SIZE => self.read_mixer(addr - bar0, data), a if a >= bar1 && a < bar1 + MASTER_REGS_SIZE => { @@ -215,8 +215,8 @@ impl PciDevice for Ac97Dev { } fn write_bar(&mut self, addr: u64, data: &[u8]) { - let bar0 = u64::from(self.config_regs.get_bar_addr(0)); - let bar1 = u64::from(self.config_regs.get_bar_addr(1)); + let bar0 = self.config_regs.get_bar_addr(0); + let bar1 = self.config_regs.get_bar_addr(1); match addr { a if a >= bar0 && a < bar0 + MIXER_REGS_SIZE => self.write_mixer(addr - bar0, data), a if a >= bar1 && a < bar1 + MASTER_REGS_SIZE => { diff --git a/devices/src/pci/pci_configuration.rs b/devices/src/pci/pci_configuration.rs index 5e268fe17d..0deee54a8d 100644 --- a/devices/src/pci/pci_configuration.rs +++ b/devices/src/pci/pci_configuration.rs @@ -177,7 +177,7 @@ pub struct PciConfiguration { } /// See pci_regs.h in kernel -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum PciBarRegionType { Memory32BitRegion = 0, IORegion = 0x01, @@ -430,11 +430,38 @@ impl PciConfiguration { Ok(config.reg_idx) } + /// Returns the type of the given BAR region. + pub fn get_bar_type(&self, bar_num: usize) -> Option { + let reg_idx = BAR0_REG + bar_num; + let reg_value = self.registers.get(reg_idx)?; + + match (reg_value & 1, (reg_value >> 1u32) & 3) { + (1, _) => Some(PciBarRegionType::IORegion), + (0, 0b00) => Some(PciBarRegionType::Memory32BitRegion), + (0, 0b10) => Some(PciBarRegionType::Memory64BitRegion), + _ => None, + } + } + /// Returns the address of the given BAR region. - pub fn get_bar_addr(&self, bar_num: usize) -> u32 { + pub fn get_bar_addr(&self, bar_num: usize) -> u64 { let bar_idx = BAR0_REG + bar_num; - self.registers[bar_idx] & BAR_MEM_ADDR_MASK + let bar_type = match self.get_bar_type(bar_num) { + Some(t) => t, + None => return 0, + }; + + match bar_type { + PciBarRegionType::IORegion => u64::from(self.registers[bar_idx] & BAR_IO_ADDR_MASK), + PciBarRegionType::Memory32BitRegion => { + u64::from(self.registers[bar_idx] & BAR_MEM_ADDR_MASK) + } + PciBarRegionType::Memory64BitRegion => { + u64::from(self.registers[bar_idx] & BAR_MEM_ADDR_MASK) + | u64::from(self.registers[bar_idx + 1]) << 32 + } + } } /// Configures the IRQ line and pin used by this device. @@ -667,4 +694,94 @@ mod tests { // The original vendor and device ID should remain. assert_eq!(cfg.read_reg(0), 0x56781234); } + + #[test] + fn add_pci_bar_mem_64bit() { + let mut cfg = PciConfiguration::new( + 0x1234, + 0x5678, + PciClassCode::MultimediaController, + &PciMultimediaSubclass::AudioController, + Some(&TestPI::Test), + PciHeaderType::Device, + 0xABCD, + 0x2468, + ); + + cfg.add_pci_bar( + &PciBarConfiguration::new( + 0, + 0x4, + PciBarRegionType::Memory64BitRegion, + PciBarPrefetchable::NotPrefetchable, + ) + .set_address(0x01234567_89ABCDE0), + ) + .expect("add_pci_bar failed"); + + assert_eq!( + cfg.get_bar_type(0), + Some(PciBarRegionType::Memory64BitRegion) + ); + assert_eq!(cfg.get_bar_addr(0), 0x01234567_89ABCDE0); + } + + #[test] + fn add_pci_bar_mem_32bit() { + let mut cfg = PciConfiguration::new( + 0x1234, + 0x5678, + PciClassCode::MultimediaController, + &PciMultimediaSubclass::AudioController, + Some(&TestPI::Test), + PciHeaderType::Device, + 0xABCD, + 0x2468, + ); + + cfg.add_pci_bar( + &PciBarConfiguration::new( + 0, + 0x4, + PciBarRegionType::Memory32BitRegion, + PciBarPrefetchable::NotPrefetchable, + ) + .set_address(0x12345670), + ) + .expect("add_pci_bar failed"); + + assert_eq!( + cfg.get_bar_type(0), + Some(PciBarRegionType::Memory32BitRegion) + ); + assert_eq!(cfg.get_bar_addr(0), 0x12345670); + } + + #[test] + fn add_pci_bar_io() { + let mut cfg = PciConfiguration::new( + 0x1234, + 0x5678, + PciClassCode::MultimediaController, + &PciMultimediaSubclass::AudioController, + Some(&TestPI::Test), + PciHeaderType::Device, + 0xABCD, + 0x2468, + ); + + cfg.add_pci_bar( + &PciBarConfiguration::new( + 0, + 0x4, + PciBarRegionType::IORegion, + PciBarPrefetchable::NotPrefetchable, + ) + .set_address(0x1230), + ) + .expect("add_pci_bar failed"); + + assert_eq!(cfg.get_bar_type(0), Some(PciBarRegionType::IORegion)); + assert_eq!(cfg.get_bar_addr(0), 0x1230); + } } diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs index cbe56a23cd..43b3a744ab 100644 --- a/devices/src/virtio/virtio_pci_device.rs +++ b/devices/src/virtio/virtio_pci_device.rs @@ -458,7 +458,7 @@ impl PciDevice for VirtioPciDevice { } fn ioeventfds(&self) -> Vec<(&EventFd, u64, Datamatch)> { - let bar0 = self.config_regs.get_bar_addr(self.settings_bar as usize) as u64; + let bar0 = self.config_regs.get_bar_addr(self.settings_bar as usize); let notify_base = bar0 + NOTIFICATION_BAR_OFFSET; self.queue_evts() .iter() @@ -504,7 +504,7 @@ impl PciDevice for VirtioPciDevice { #[allow(clippy::absurd_extreme_comparisons)] 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) as u64; + let bar0 = self.config_regs.get_bar_addr(self.settings_bar as usize); let offset = addr - bar0; match offset { o if COMMON_CONFIG_BAR_OFFSET <= o @@ -556,7 +556,7 @@ impl PciDevice for VirtioPciDevice { #[allow(clippy::absurd_extreme_comparisons)] fn write_bar(&mut self, addr: u64, data: &[u8]) { - let bar0 = self.config_regs.get_bar_addr(self.settings_bar as usize) as u64; + let bar0 = self.config_regs.get_bar_addr(self.settings_bar as usize); let offset = addr - bar0; match offset { o if COMMON_CONFIG_BAR_OFFSET <= o