From 123eef963572f07d55a19167c2d4ea41dfd15054 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 21:34:48 +0200 Subject: [PATCH 1/6] pcid: Move capability offset into the Capability enum This makes it easier to move to using pci_types for parsing PCI capabilities. --- pcid/src/driver_handler.rs | 50 ++++++++++++++++------------------ pcid/src/pci/cap.rs | 10 +++++-- pcid/src/pci/msi.rs | 56 +++++++++++++++++++++++--------------- 3 files changed, 66 insertions(+), 50 deletions(-) diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index 60c893dd85..4ecb55a4c7 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -14,7 +14,7 @@ use crate::State; pub struct DriverHandler { addr: PciAddress, - capabilities: Vec<(u8, PciCapability)>, + capabilities: Vec, state: Arc, } @@ -23,7 +23,7 @@ impl DriverHandler { pub fn spawn( state: Arc, func: driver_interface::PciFunction, - capabilities: Vec<(u8, PciCapability)>, + capabilities: Vec, args: &[String], ) { let subdriver_args = driver_interface::SubdriverArguments { func }; @@ -110,14 +110,14 @@ impl DriverHandler { PcidClientRequest::RequestCapabilities => PcidClientResponse::Capabilities( self.capabilities .iter() - .map(|(_, capability)| capability.clone()) + .map(|capability| capability.clone()) .collect::>(), ), PcidClientRequest::RequestConfig => PcidClientResponse::Config(args.clone()), PcidClientRequest::RequestFeatures => PcidClientResponse::AllFeatures( self.capabilities .iter() - .filter_map(|(_, capability)| match capability { + .filter_map(|capability| match capability { PciCapability::Msi(msi) => { Some((PciFeature::Msi, FeatureStatus::enabled(msi.enabled()))) } @@ -131,13 +131,12 @@ impl DriverHandler { ), PcidClientRequest::EnableFeature(feature) => match feature { PciFeature::Msi => { - let (offset, capability): (u8, &mut MsiCapability) = match self + let capability: &mut MsiCapability = match self .capabilities .iter_mut() - .find_map(|&mut (offset, ref mut capability)| { - capability.as_msi_mut().map(|cap| (offset, cap)) - }) { - Some(tuple) => tuple, + .find_map(|capability| capability.as_msi_mut()) + { + Some(capability) => capability, None => { return PcidClientResponse::Error( PcidServerResponseError::NonexistentFeature(feature), @@ -146,18 +145,17 @@ impl DriverHandler { }; unsafe { capability.set_enabled(true); - capability.write_message_control(&func, offset); + capability.write_message_control(&func); } PcidClientResponse::FeatureEnabled(feature) } PciFeature::MsiX => { - let (offset, capability): (u8, &mut MsixCapability) = match self + let capability: &mut MsixCapability = match self .capabilities .iter_mut() - .find_map(|&mut (offset, ref mut capability)| { - capability.as_msix_mut().map(|cap| (offset, cap)) - }) { - Some(tuple) => tuple, + .find_map(|capability| capability.as_msix_mut()) + { + Some(capability) => capability, None => { return PcidClientResponse::Error( PcidServerResponseError::NonexistentFeature(feature), @@ -166,7 +164,7 @@ impl DriverHandler { }; unsafe { capability.set_msix_enabled(true); - capability.write_a(&func, offset); + capability.write_a(&func); } PcidClientResponse::FeatureEnabled(feature) } @@ -177,7 +175,7 @@ impl DriverHandler { PciFeature::Msi => self .capabilities .iter() - .find_map(|(_, capability)| { + .find_map(|capability| { if let PciCapability::Msi(msi) = capability { Some(FeatureStatus::enabled(msi.enabled())) } else { @@ -188,7 +186,7 @@ impl DriverHandler { PciFeature::MsiX => self .capabilities .iter() - .find_map(|(_, capability)| { + .find_map(|capability| { if let PciCapability::MsiX(msix) = capability { Some(FeatureStatus::enabled(msix.msix_enabled())) } else { @@ -205,7 +203,7 @@ impl DriverHandler { if let Some(info) = self .capabilities .iter() - .find_map(|(_, capability)| capability.as_msi()) + .find_map(|capability| capability.as_msi()) { PciFeatureInfo::Msi(*info) } else { @@ -218,7 +216,7 @@ impl DriverHandler { if let Some(info) = self .capabilities .iter() - .find_map(|(_, capability)| capability.as_msix()) + .find_map(|capability| capability.as_msix()) { PciFeatureInfo::MsiX(*info) } else { @@ -231,10 +229,10 @@ impl DriverHandler { ), PcidClientRequest::SetFeatureInfo(info_to_set) => match info_to_set { SetFeatureInfo::Msi(info_to_set) => { - if let Some((offset, info)) = self + if let Some(info) = self .capabilities .iter_mut() - .find_map(|(offset, capability)| Some((*offset, capability.as_msi_mut()?))) + .find_map(|capability| capability.as_msi_mut()) { if let Some(mme) = info_to_set.multi_message_enable { if info.multi_message_capable() < mme || mme > 0b101 { @@ -271,7 +269,7 @@ impl DriverHandler { info.set_mask_bits(mask_bits); } unsafe { - info.write_all(&func, offset); + info.write_all(&func); } PcidClientResponse::SetFeatureInfo(PciFeature::Msi) } else { @@ -281,15 +279,15 @@ impl DriverHandler { } } SetFeatureInfo::MsiX { function_mask } => { - if let Some((offset, info)) = self + if let Some(info) = self .capabilities .iter_mut() - .find_map(|(offset, capability)| Some((*offset, capability.as_msix_mut()?))) + .find_map(|capability| capability.as_msix_mut()) { if let Some(mask) = function_mask { info.set_function_mask(mask); unsafe { - info.write_a(&func, offset); + info.write_a(&func); } } PcidClientResponse::SetFeatureInfo(PciFeature::MsiX) diff --git a/pcid/src/pci/cap.rs b/pcid/src/pci/cap.rs index 21ce506f00..06e1436f5f 100644 --- a/pcid/src/pci/cap.rs +++ b/pcid/src/pci/cap.rs @@ -15,7 +15,7 @@ impl<'a> CapabilitiesIter<'a> { } } impl<'a> Iterator for CapabilitiesIter<'a> { - type Item = (u8, Capability); + type Item = Capability; fn next(&mut self) -> Option { let offset = unsafe { @@ -37,7 +37,7 @@ impl<'a> Iterator for CapabilitiesIter<'a> { Capability::parse(self.func, offset) }; - Some((offset, cap)) + Some(cap) } } @@ -56,17 +56,20 @@ pub enum CapabilityId { #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] pub enum MsiCapability { _32BitAddress { + cap_offset: u8, message_control: u32, message_address: u32, message_data: u16, }, _64BitAddress { + cap_offset: u8, message_control: u32, message_address_lo: u32, message_address_hi: u32, message_data: u16, }, _32BitAddressWithPvm { + cap_offset: u8, message_control: u32, message_address: u32, message_data: u32, @@ -74,6 +77,7 @@ pub enum MsiCapability { pending_bits: u32, }, _64BitAddressWithPvm { + cap_offset: u8, message_control: u32, message_address_lo: u32, message_address_hi: u32, @@ -85,6 +89,7 @@ pub enum MsiCapability { #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] pub struct MsixCapability { + pub cap_offset: u8, pub a: u32, pub b: u32, pub c: u32, @@ -133,6 +138,7 @@ impl Capability { } unsafe fn parse_msix(func: &PciFunc, offset: u8) -> Self { Self::MsiX(MsixCapability { + cap_offset: offset, a: func.read_u32(u16::from(offset)), b: func.read_u32(u16::from(offset + 4)), c: func.read_u32(u16::from(offset + 8)), diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index 8d6e8f4f83..ce9b35cb9e 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -43,6 +43,7 @@ impl MsiCapability { if message_control & Self::MC_PVT_CAPABLE_BIT != 0 { if message_control & Self::MC_64_BIT_ADDR_BIT != 0 { Self::_64BitAddressWithPvm { + cap_offset: offset, message_control: dword, message_address_lo: func.read_u32(u16::from(offset + 4)), message_address_hi: func.read_u32(u16::from(offset + 8)), @@ -52,6 +53,7 @@ impl MsiCapability { } } else { Self::_32BitAddressWithPvm { + cap_offset: offset, message_control: dword, message_address: func.read_u32(u16::from(offset + 4)), message_data: func.read_u32(u16::from(offset + 8)), @@ -62,6 +64,7 @@ impl MsiCapability { } else { if message_control & Self::MC_64_BIT_ADDR_BIT != 0 { Self::_64BitAddress { + cap_offset: offset, message_control: dword, message_address_lo: func.read_u32(u16::from(offset + 4)), message_address_hi: func.read_u32(u16::from(offset + 8)), @@ -69,6 +72,7 @@ impl MsiCapability { } } else { Self::_32BitAddress { + cap_offset: offset, message_control: dword, message_address: func.read_u32(u16::from(offset + 4)), message_data: func.read_u32(u16::from(offset + 8)) as u16, @@ -76,6 +80,14 @@ impl MsiCapability { } } } + fn cap_offset(&self) -> u16 { + match *self { + MsiCapability::_32BitAddress { cap_offset, .. } + | MsiCapability::_64BitAddress { cap_offset, .. } + | MsiCapability::_32BitAddressWithPvm { cap_offset, .. } + | MsiCapability::_64BitAddressWithPvm { cap_offset, .. } => u16::from(cap_offset), + } + } fn message_control_raw(&self) -> u32 { match self { @@ -97,8 +109,8 @@ impl MsiCapability { | Self::_64BitAddressWithPvm { ref mut message_control, .. } => *message_control = new_message_control, } } - pub unsafe fn write_message_control(&self, func: &PciFunc, offset: u8) { - func.write_u32(u16::from(offset), self.message_control_raw()); + pub unsafe fn write_message_control(&self, func: &PciFunc) { + func.write_u32(self.cap_offset(), self.message_control_raw()); } pub fn is_pvt_capable(&self) -> bool { self.message_control() & Self::MC_PVT_CAPABLE_BIT != 0 @@ -186,36 +198,36 @@ impl MsiCapability { } Some(()) } - pub unsafe fn write_message_address(&self, func: &PciFunc, offset: u8) { - func.write_u32(u16::from(offset) + 4, self.message_address()) + pub unsafe fn write_message_address(&self, func: &PciFunc) { + func.write_u32(self.cap_offset() + 4, self.message_address()) } - pub unsafe fn write_message_upper_address(&self, func: &PciFunc, offset: u8) -> Option<()> { + pub unsafe fn write_message_upper_address(&self, func: &PciFunc) -> Option<()> { let value = self.message_upper_address()?; - func.write_u32(u16::from(offset + 8), value); + func.write_u32(self.cap_offset() + 8, value); Some(()) } - pub unsafe fn write_message_data(&self, func: &PciFunc, offset: u8) { + pub unsafe fn write_message_data(&self, func: &PciFunc) { match self { - &Self::_32BitAddress { message_data, .. } => func.write_u32(u16::from(offset + 8), message_data.into()), - &Self::_32BitAddressWithPvm { message_data, .. } => func.write_u32(u16::from(offset + 8), message_data), - &Self::_64BitAddress { message_data, .. } => func.write_u32(u16::from(offset + 12), message_data.into()), - &Self::_64BitAddressWithPvm { message_data, .. } => func.write_u32(u16::from(offset + 12), message_data), + &Self::_32BitAddress { cap_offset, message_data, .. } => func.write_u32(u16::from(cap_offset + 8), message_data.into()), + &Self::_32BitAddressWithPvm { cap_offset, message_data, .. } => func.write_u32(u16::from(cap_offset + 8), message_data), + &Self::_64BitAddress { cap_offset, message_data, .. } => func.write_u32(u16::from(cap_offset + 12), message_data.into()), + &Self::_64BitAddressWithPvm { cap_offset, message_data, .. } => func.write_u32(u16::from(cap_offset + 12), message_data), } } - pub unsafe fn write_mask_bits(&self, func: &PciFunc, offset: u8) -> Option<()> { + pub unsafe fn write_mask_bits(&self, func: &PciFunc) -> Option<()> { match self { - &Self::_32BitAddressWithPvm { mask_bits, .. } => func.write_u32(u16::from(offset + 12), mask_bits), - &Self::_64BitAddressWithPvm { mask_bits, .. } => func.write_u32(u16::from(offset + 16), mask_bits), + &Self::_32BitAddressWithPvm { cap_offset, mask_bits, .. } => func.write_u32(u16::from(cap_offset + 12), mask_bits), + &Self::_64BitAddressWithPvm { cap_offset, mask_bits, .. } => func.write_u32(u16::from(cap_offset + 16), mask_bits), &Self::_32BitAddress { .. } | &Self::_64BitAddress { .. } => return None, } Some(()) } - pub unsafe fn write_all(&self, func: &PciFunc, offset: u8) { - self.write_message_control(func, offset); - self.write_message_address(func, offset); - self.write_message_upper_address(func, offset); - self.write_message_data(func, offset); - self.write_mask_bits(func, offset); + pub unsafe fn write_all(&self, func: &PciFunc) { + self.write_message_control(func); + self.write_message_address(func); + self.write_message_upper_address(func); + self.write_message_data(func); + self.write_mask_bits(func); } } @@ -329,8 +341,8 @@ impl MsixCapability { /// Write the first DWORD into configuration space (containing the partially modifiable Message /// Control field). - pub unsafe fn write_a(&self, func: &PciFunc, offset: u8) { - func.write_u32(u16::from(offset), self.a) + pub unsafe fn write_a(&self, func: &PciFunc) { + func.write_u32(u16::from(self.cap_offset), self.a) } } From e14e56349f436d5f4e18f614f7629c2afb21ada2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 21:40:43 +0200 Subject: [PATCH 2/6] pcid: Remove method to get the current feature status Devices should never change the feature status behind the back of the driver, so storing the current status after setting it is fine. Drivers should reset all state when they start to recover from a crashed driver, so they shouldn't need to get the current feature status at startup either. --- pcid/src/driver_handler.rs | 27 --------------------------- pcid/src/driver_interface/mod.rs | 8 -------- 2 files changed, 35 deletions(-) diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index 4ecb55a4c7..741e374fe4 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -169,33 +169,6 @@ impl DriverHandler { PcidClientResponse::FeatureEnabled(feature) } }, - PcidClientRequest::FeatureStatus(feature) => PcidClientResponse::FeatureStatus( - feature, - match feature { - PciFeature::Msi => self - .capabilities - .iter() - .find_map(|capability| { - if let PciCapability::Msi(msi) = capability { - Some(FeatureStatus::enabled(msi.enabled())) - } else { - None - } - }) - .unwrap_or(FeatureStatus::Disabled), - PciFeature::MsiX => self - .capabilities - .iter() - .find_map(|capability| { - if let PciCapability::MsiX(msix) = capability { - Some(FeatureStatus::enabled(msix.msix_enabled())) - } else { - None - } - }) - .unwrap_or(FeatureStatus::Disabled), - }, - ), PcidClientRequest::FeatureInfo(feature) => PcidClientResponse::FeatureInfo( feature, match feature { diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index aa03a9d587..32e324ec4e 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -194,7 +194,6 @@ pub enum PcidClientRequest { RequestFeatures, RequestCapabilities, EnableFeature(PciFeature), - FeatureStatus(PciFeature), FeatureInfo(PciFeature), SetFeatureInfo(SetFeatureInfo), ReadConfig(u16), @@ -295,13 +294,6 @@ impl PcidServerHandle { other => Err(PcidClientHandleError::InvalidResponse(other)), } } - pub fn feature_status(&mut self, feature: PciFeature) -> Result { - self.send(&PcidClientRequest::FeatureStatus(feature))?; - match self.recv()? { - PcidClientResponse::FeatureStatus(feat, status) if feat == feature => Ok(status), - other => Err(PcidClientHandleError::InvalidResponse(other)), - } - } pub fn enable_feature(&mut self, feature: PciFeature) -> Result<()> { self.send(&PcidClientRequest::EnableFeature(feature))?; match self.recv()? { From 588bbfe6a3ec25ed3716dd7f0cc1e46774477854 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 9 Jun 2024 21:54:36 +0200 Subject: [PATCH 3/6] pcid: Stop returning feature enable status from fetch_all_features For the same reason the feature_status method was removed. --- audio/ihdad/src/main.rs | 13 +++---------- net/rtl8139d/src/main.rs | 15 ++++----------- net/rtl8168d/src/main.rs | 15 ++++----------- pcid/src/driver_handler.rs | 9 ++------- pcid/src/driver_interface/mod.rs | 4 ++-- storage/nvmed/src/main.rs | 4 ++-- virtio-core/src/arch/x86.rs | 4 +--- virtio-core/src/arch/x86_64.rs | 4 +--- virtio-core/src/probe.rs | 7 +++---- xhcid/src/main.rs | 15 ++++----------- 10 files changed, 26 insertions(+), 64 deletions(-) diff --git a/audio/ihdad/src/main.rs b/audio/ihdad/src/main.rs index bab7c5b6df..e86cc832a4 100755 --- a/audio/ihdad/src/main.rs +++ b/audio/ihdad/src/main.rs @@ -82,17 +82,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { let all_pci_features = pcid_handle.fetch_all_features().expect("ihdad: failed to fetch pci features"); log::debug!("PCI FEATURES: {:?}", all_pci_features); - let (has_msi, mut msi_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msi(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); - let (has_msix, mut msix_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msix(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); + let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); + let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); - if has_msi && !msi_enabled && !has_msix { - msi_enabled = true; - } - if has_msix && !msix_enabled { - msix_enabled = true; - } - - if msi_enabled && !msix_enabled { + if has_msi && !has_msix { let capability = match pcid_handle.feature_info(PciFeature::Msi).expect("ihdad: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), diff --git a/net/rtl8139d/src/main.rs b/net/rtl8139d/src/main.rs index b04e5a7829..fc51f4653b 100644 --- a/net/rtl8139d/src/main.rs +++ b/net/rtl8139d/src/main.rs @@ -101,17 +101,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { let all_pci_features = pcid_handle.fetch_all_features().expect("rtl8139d: failed to fetch pci features"); log::info!("PCI FEATURES: {:?}", all_pci_features); - let (has_msi, mut msi_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msi(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); - let (has_msix, mut msix_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msix(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); + let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); + let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); - if has_msi && !msi_enabled && !has_msix { - msi_enabled = true; - } - if has_msix && !msix_enabled { - msix_enabled = true; - } - - if msi_enabled && !msix_enabled { + if has_msi && !has_msix { let capability = match pcid_handle.feature_info(PciFeature::Msi).expect("rtl8139d: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), @@ -135,7 +128,7 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { log::info!("Enabled MSI"); interrupt_handle - } else if msix_enabled { + } else if has_msix { let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8139d: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, diff --git a/net/rtl8168d/src/main.rs b/net/rtl8168d/src/main.rs index e2d594948e..d13cc10d80 100644 --- a/net/rtl8168d/src/main.rs +++ b/net/rtl8168d/src/main.rs @@ -96,17 +96,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { let all_pci_features = pcid_handle.fetch_all_features().expect("rtl8168d: failed to fetch pci features"); log::info!("PCI FEATURES: {:?}", all_pci_features); - let (has_msi, mut msi_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msi(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); - let (has_msix, mut msix_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msix(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); + let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); + let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); - if has_msi && !msi_enabled && !has_msix { - msi_enabled = true; - } - if has_msix && !msix_enabled { - msix_enabled = true; - } - - if msi_enabled && !msix_enabled { + if has_msi && !has_msix { let capability = match pcid_handle.feature_info(PciFeature::Msi).expect("rtl8168d: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), @@ -130,7 +123,7 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { log::info!("Enabled MSI"); interrupt_handle - } else if msix_enabled { + } else if has_msix { let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8168d: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index 741e374fe4..210cf9eefe 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -118,13 +118,8 @@ impl DriverHandler { self.capabilities .iter() .filter_map(|capability| match capability { - PciCapability::Msi(msi) => { - Some((PciFeature::Msi, FeatureStatus::enabled(msi.enabled()))) - } - PciCapability::MsiX(msix) => Some(( - PciFeature::MsiX, - FeatureStatus::enabled(msix.msix_enabled()), - )), + PciCapability::Msi(_) => Some(PciFeature::Msi), + PciCapability::MsiX(_) => Some(PciFeature::MsiX), _ => None, }) .collect(), diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index 32e324ec4e..d7c1cdf16a 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -212,7 +212,7 @@ pub enum PcidServerResponseError { pub enum PcidClientResponse { Capabilities(Vec), Config(SubdriverArguments), - AllFeatures(Vec<(PciFeature, FeatureStatus)>), + AllFeatures(Vec), FeatureEnabled(PciFeature), FeatureStatus(PciFeature, FeatureStatus), Error(PcidServerResponseError), @@ -287,7 +287,7 @@ impl PcidServerHandle { } // FIXME turn into struct with bool fields - pub fn fetch_all_features(&mut self) -> Result> { + pub fn fetch_all_features(&mut self) -> Result> { self.send(&PcidClientRequest::RequestFeatures)?; match self.recv()? { PcidClientResponse::AllFeatures(a) => Ok(a), diff --git a/storage/nvmed/src/main.rs b/storage/nvmed/src/main.rs index 3ca9b0e22c..dd9c1682c8 100644 --- a/storage/nvmed/src/main.rs +++ b/storage/nvmed/src/main.rs @@ -76,8 +76,8 @@ fn get_int_method( let features = pcid_handle.fetch_all_features().unwrap(); - let has_msi = features.iter().any(|(feature, _)| feature.is_msi()); - let has_msix = features.iter().any(|(feature, _)| feature.is_msix()); + let has_msi = features.iter().any(|feature| feature.is_msi()); + let has_msix = features.iter().any(|feature| feature.is_msix()); // TODO: Allocate more than one vector when possible and useful. if has_msix { diff --git a/virtio-core/src/arch/x86.rs b/virtio-core/src/arch/x86.rs index 23fdbbf3a0..be993050c5 100644 --- a/virtio-core/src/arch/x86.rs +++ b/virtio-core/src/arch/x86.rs @@ -20,9 +20,7 @@ pub fn probe_legacy_port_transport( // Setup interrupts. let all_pci_features = pcid_handle.fetch_all_features()?; - let has_msix = all_pci_features - .iter() - .any(|(feature, _)| feature.is_msix()); + let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); // According to the virtio specification, the device REQUIRED to support MSI-X. assert!(has_msix, "virtio: device does not support MSI-X"); diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index fa0de00c6c..7b95c4fe84 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -61,9 +61,7 @@ pub fn probe_legacy_port_transport( // Setup interrupts. let all_pci_features = pcid_handle.fetch_all_features()?; - let has_msix = all_pci_features - .iter() - .any(|(feature, _)| feature.is_msix()); + let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); // According to the virtio specification, the device REQUIRED to support MSI-X. assert!(has_msix, "virtio: device does not support MSI-X"); diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index aebd065d04..e61898f848 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -123,7 +123,8 @@ pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result // SAFETY: The capability type is `Notify`, so its safe to access // the `notify_multiplier` field. let multiplier = unsafe { - (&*(raw_capability.data.as_ptr() as *const PciCapability as *const PciCapabilityNotify)) + (&*(raw_capability.data.as_ptr() as *const PciCapability + as *const PciCapabilityNotify)) .notify_off_multiplier() }; notify_addr = Some((address, multiplier)); @@ -161,9 +162,7 @@ pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result // Setup interrupts. let all_pci_features = pcid_handle.fetch_all_features()?; - let has_msix = all_pci_features - .iter() - .any(|(feature, _)| feature.is_msix()); + let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); // According to the virtio specification, the device REQUIRED to support MSI-X. assert!(has_msix, "virtio: device does not support MSI-X"); diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 6a10660160..047e16e148 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -91,17 +91,10 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O let all_pci_features = pcid_handle.fetch_all_features().expect("xhcid: failed to fetch pci features"); log::debug!("XHCI PCI FEATURES: {:?}", all_pci_features); - let (has_msi, mut msi_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msi(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); - let (has_msix, mut msix_enabled) = all_pci_features.iter().map(|(feature, status)| (feature.is_msix(), status.is_enabled())).find(|&(f, _)| f).unwrap_or((false, false)); + let has_msi = all_pci_features.iter().any(|feature| feature.is_msi()); + let has_msix = all_pci_features.iter().any(|feature| feature.is_msix()); - if has_msi && !msi_enabled && !has_msix { - msi_enabled = true; - } - if has_msix && !msix_enabled { - msix_enabled = true; - } - - if msi_enabled && !msix_enabled { + if has_msi && !has_msix { let mut capability = match pcid_handle.feature_info(PciFeature::Msi).expect("xhcid: failed to retrieve the MSI capability structure from pcid") { PciFeatureInfo::Msi(s) => s, PciFeatureInfo::MsiX(_) => panic!(), @@ -125,7 +118,7 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O log::debug!("Enabled MSI"); (Some(interrupt_handle), InterruptMethod::Msi) - } else if msix_enabled { + } else if has_msix { let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("xhcid: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, From 15c9ad1ba6f485323c261ca71931b8eaa25d99d2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 10 Jun 2024 13:33:40 +0200 Subject: [PATCH 4/6] pcid: Ensure MSI and MSI-X are not enabled at the same time --- pcid/src/driver_handler.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index 210cf9eefe..c809184d9d 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -126,6 +126,19 @@ impl DriverHandler { ), PcidClientRequest::EnableFeature(feature) => match feature { PciFeature::Msi => { + if let Some(msix_capability) = self + .capabilities + .iter_mut() + .find_map(|capability| capability.as_msix_mut()) + { + // If MSI-X is supported disable it before enabling MSI as they can't be + // active at the same time. + unsafe { + msix_capability.set_msix_enabled(false); + msix_capability.write_a(&func); + } + } + let capability: &mut MsiCapability = match self .capabilities .iter_mut() @@ -145,6 +158,19 @@ impl DriverHandler { PcidClientResponse::FeatureEnabled(feature) } PciFeature::MsiX => { + if let Some(msi_capability) = self + .capabilities + .iter_mut() + .find_map(|capability| capability.as_msi_mut()) + { + // If MSI is supported disable it before enabling MSI-X as they can't be + // active at the same time. + unsafe { + msi_capability.set_enabled(false); + msi_capability.write_message_control(&func); + } + } + let capability: &mut MsixCapability = match self .capabilities .iter_mut() From 7c112c34dd7fba1134502dfa1784b242f8ef9f7f Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 10 Jun 2024 13:46:14 +0200 Subject: [PATCH 5/6] pcid: Remove some unused methods on MsixCapability --- pcid/src/pci/msi.rs | 18 ++++-------------- storage/nvmed/src/main.rs | 1 - 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index ce9b35cb9e..ac042985c9 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -279,11 +279,11 @@ impl MsixCapability { /// The Message Control field, containing the enabled and function mask bits, as well as the /// table size. - pub const fn message_control(&self) -> u16 { + const fn message_control(&self) -> u16 { (self.a >> 16) as u16 } - pub fn set_message_control(&mut self, message_control: u16) { + pub(crate) fn set_message_control(&mut self, message_control: u16) { self.a &= 0x0000_FFFF; self.a |= u32::from(message_control) << 16; } @@ -291,24 +291,14 @@ impl MsixCapability { pub const fn table_size(&self) -> u16 { (self.message_control() & Self::MC_TABLE_SIZE_MASK) + 1 } - /// Returns the MSI-X enabled bit, which enables MSI-X if the MSI enable bit is also set in the - /// MSI capability structure. - pub const fn msix_enabled(&self) -> bool { - self.message_control() & Self::MC_MSIX_ENABLED_BIT != 0 - } - /// The MSI-X function mask, which overrides each of the vectors' mask bit, when set. - pub const fn function_mask(&self) -> bool { - self.message_control() & Self::MC_FUNCTION_MASK_BIT != 0 - } - - pub fn set_msix_enabled(&mut self, enabled: bool) { + pub(crate) fn set_msix_enabled(&mut self, enabled: bool) { let mut new_message_control = self.message_control(); new_message_control &= !(Self::MC_MSIX_ENABLED_BIT); new_message_control |= u16::from(enabled) << Self::MC_MSIX_ENABLED_SHIFT; self.set_message_control(new_message_control); } - pub fn set_function_mask(&mut self, function_mask: bool) { + pub(crate) fn set_function_mask(&mut self, function_mask: bool) { let mut new_message_control = self.message_control(); new_message_control &= !(Self::MC_FUNCTION_MASK_BIT); new_message_control |= u16::from(function_mask) << Self::MC_FUNCTION_MASK_SHIFT; diff --git a/storage/nvmed/src/main.rs b/storage/nvmed/src/main.rs index dd9c1682c8..30d0b2788a 100644 --- a/storage/nvmed/src/main.rs +++ b/storage/nvmed/src/main.rs @@ -125,7 +125,6 @@ fn get_int_method( } pcid_handle.enable_feature(PciFeature::MsiX).unwrap(); - capability_struct.set_msix_enabled(true); // only affects our local mirror of the cap let (msix_vector_number, irq_handle) = { let entry: &mut MsixTableEntry = &mut table_entries[0]; From df666331116c26f9d6d2c6007afc015293df9802 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 10 Jun 2024 21:00:07 +0200 Subject: [PATCH 6/6] pcid: Store decoded MsixInfo instead of full capability in PciFeatureInfo --- net/rtl8139d/src/main.rs | 22 +++++++++++----------- net/rtl8168d/src/main.rs | 22 +++++++++++----------- pcid/src/driver_handler.rs | 8 +++++++- pcid/src/driver_interface/mod.rs | 2 +- pcid/src/pci/msi.rs | 31 +++++++++++++++++++++---------- storage/nvmed/src/main.rs | 16 ++++++++-------- storage/nvmed/src/nvme/mod.rs | 8 ++++---- virtio-core/src/arch/x86_64.rs | 14 +++++++------- virtio-core/src/probe.rs | 10 +++++----- xhcid/src/main.rs | 12 ++++++------ xhcid/src/xhci/mod.rs | 16 ++++++++-------- 11 files changed, 89 insertions(+), 72 deletions(-) diff --git a/net/rtl8139d/src/main.rs b/net/rtl8139d/src/main.rs index fc51f4653b..a949d46e18 100644 --- a/net/rtl8139d/src/main.rs +++ b/net/rtl8139d/src/main.rs @@ -13,7 +13,7 @@ use event::{user_data, EventQueue}; #[cfg(target_arch = "x86_64")] use pcid_interface::irq_helpers::allocate_single_interrupt_vector_for_msi; use pcid_interface::irq_helpers::read_bsp_apic_id; -use pcid_interface::msi::{MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::{ MsiSetFeatureInfo, PciFeature, PciFeatureInfo, PcidServerHandle, SetFeatureInfo, SubdriverArguments, @@ -79,17 +79,17 @@ where } } -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl MsixInfo { +impl MappedMsixRegs { pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { &mut *self.virt_table_base.as_ptr().offset(k as isize) } pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } @@ -129,20 +129,20 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { interrupt_handle } else if has_msix { - let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8139d: failed to retrieve the MSI-X capability structure from pcid") { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8139d: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar = &pci_config.func.bars[msix_info.table_bar as usize]; let bar_address = unsafe { bar.physmap_mem("rtl8139d") } as usize; - let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - let mut info = MsixInfo { + let mut info = MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate one msi vector. diff --git a/net/rtl8168d/src/main.rs b/net/rtl8168d/src/main.rs index d13cc10d80..c797c2e059 100644 --- a/net/rtl8168d/src/main.rs +++ b/net/rtl8168d/src/main.rs @@ -12,7 +12,7 @@ use pcid_interface::{MsiSetFeatureInfo, PcidServerHandle, PciFeature, PciFeature #[cfg(target_arch = "x86_64")] use pcid_interface::irq_helpers::allocate_single_interrupt_vector_for_msi; use pcid_interface::irq_helpers::read_bsp_apic_id; -use pcid_interface::msi::{MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use redox_log::{RedoxLogger, OutputBuilder}; use syscall::EventFlags; @@ -74,17 +74,17 @@ where } } -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl MsixInfo { +impl MappedMsixRegs { pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { &mut *self.virt_table_base.as_ptr().offset(k as isize) } pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } @@ -124,20 +124,20 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> File { interrupt_handle } else if has_msix { - let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8168d: failed to retrieve the MSI-X capability structure from pcid") { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).expect("rtl8168d: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar = &pci_config.func.bars[msix_info.table_bar as usize]; let bar_address = unsafe { bar.physmap_mem("rtl8168d") } as usize; - let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - let mut info = MsixInfo { + let mut info = MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate one msi vector. diff --git a/pcid/src/driver_handler.rs b/pcid/src/driver_handler.rs index c809184d9d..3358a501fc 100644 --- a/pcid/src/driver_handler.rs +++ b/pcid/src/driver_handler.rs @@ -212,7 +212,13 @@ impl DriverHandler { .iter() .find_map(|capability| capability.as_msix()) { - PciFeatureInfo::MsiX(*info) + PciFeatureInfo::MsiX(msi::MsixInfo { + table_bar: info.table_bir(), + table_offset: info.table_offset(), + table_size: info.table_size(), + pba_bar: info.pba_bir(), + pba_offset: info.pba_offset(), + }) } else { return PcidClientResponse::Error( PcidServerResponseError::NonexistentFeature(feature), diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index d7c1cdf16a..483da35602 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -133,7 +133,7 @@ impl PciFeature { #[derive(Debug, Serialize, Deserialize)] pub enum PciFeatureInfo { Msi(msi::MsiCapability), - MsiX(msi::MsixCapability), + MsiX(msi::MsixInfo), } #[derive(Debug, Error)] diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index ac042985c9..179ae638f2 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -231,24 +231,33 @@ impl MsiCapability { } } -impl MsixCapability { +#[derive(Debug, Serialize, Deserialize)] +pub struct MsixInfo { + pub table_bar: u8, + pub table_offset: u32, + pub table_size: u16, + pub pba_bar: u8, + pub pba_offset: u32, +} + +impl MsixInfo { pub fn validate(&self, bars: [PciBar; 6]) { - if self.table_bir() > 5 { - panic!("MSI-X Table BIR contained a reserved enum value: {}", self.table_bir()); + if self.table_bar > 5 { + panic!("MSI-X Table BIR contained a reserved enum value: {}", self.table_bar); } - if self.pba_bir() > 5 { - panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bir()); + if self.pba_bar > 5 { + panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bar); } - let table_size = self.table_size(); - let table_offset = self.table_offset() as usize; + let table_size = self.table_size; + let table_offset = self.table_offset as usize; let table_min_length = table_size * 16; - let pba_offset = self.pba_offset() as usize; + let pba_offset = self.pba_offset as usize; let pba_min_length = table_size.div_ceil(8); - let (_, table_bar_size) = bars[self.table_bir() as usize].expect_mem(); - let (_, pba_bar_size) = bars[self.pba_bir() as usize].expect_mem(); + let (_, table_bar_size) = bars[self.table_bar as usize].expect_mem(); + let (_, pba_bar_size) = bars[self.pba_bar as usize].expect_mem(); // Ensure that the table and PBA are within the BAR. @@ -270,7 +279,9 @@ impl MsixCapability { ); } } +} +impl MsixCapability { const MC_MSIX_ENABLED_BIT: u16 = 1 << 15; const MC_MSIX_ENABLED_SHIFT: u8 = 15; const MC_FUNCTION_MASK_BIT: u16 = 1 << 14; diff --git a/storage/nvmed/src/main.rs b/storage/nvmed/src/main.rs index 30d0b2788a..501c1b74e8 100644 --- a/storage/nvmed/src/main.rs +++ b/storage/nvmed/src/main.rs @@ -82,14 +82,14 @@ fn get_int_method( // TODO: Allocate more than one vector when possible and useful. if has_msix { // Extended message signaled interrupts. - use self::nvme::MsixCfg; + use self::nvme::MappedMsixRegs; use pcid_interface::msi::MsixTableEntry; - let mut capability_struct = match pcid_handle.feature_info(PciFeature::MsiX).unwrap() { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).unwrap() { PciFeatureInfo::MsiX(msix) => msix, _ => unreachable!(), }; - capability_struct.validate(function.bars); + msix_info.validate(function.bars); fn bar_base( allocated_bars: &AllocatedBars, function: &PciFunction, @@ -109,11 +109,11 @@ fn get_int_method( } } let table_bar_base: *mut u8 = - bar_base(allocated_bars, function, capability_struct.table_bir())?.as_ptr(); + bar_base(allocated_bars, function, msix_info.table_bar)?.as_ptr(); let table_base = - unsafe { table_bar_base.offset(capability_struct.table_offset() as isize) }; + unsafe { table_bar_base.offset(msix_info.table_offset as isize) }; - let vector_count = capability_struct.table_size(); + let vector_count = msix_info.table_size; let table_entries: &'static mut [MsixTableEntry] = unsafe { slice::from_raw_parts_mut(table_base as *mut MsixTableEntry, vector_count as usize) }; @@ -139,8 +139,8 @@ fn get_int_method( (0, irq_handle) }; - let interrupt_method = InterruptMethod::MsiX(MsixCfg { - cap: capability_struct, + let interrupt_method = InterruptMethod::MsiX(MappedMsixRegs { + info: msix_info, table: table_entries, }); let interrupt_sources = diff --git a/storage/nvmed/src/nvme/mod.rs b/storage/nvmed/src/nvme/mod.rs index 6378fa2fce..7781f477bc 100644 --- a/storage/nvmed/src/nvme/mod.rs +++ b/storage/nvmed/src/nvme/mod.rs @@ -22,7 +22,7 @@ pub mod queues; use self::cq_reactor::NotifReq; pub use self::queues::{NvmeCmd, NvmeCmdQueue, NvmeComp, NvmeCompQueue}; -use pcid_interface::msi::{MsiCapability, MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsiCapability, MsixInfo, MsixTableEntry}; use pcid_interface::PcidServerHandle; #[cfg(target_arch = "aarch64")] @@ -93,7 +93,7 @@ pub enum InterruptMethod { /// Message signaled interrupts Msi(MsiCapability), /// Extended message signaled interrupts - MsiX(MsixCfg), + MsiX(MappedMsixRegs), } impl InterruptMethod { fn is_intx(&self) -> bool { @@ -119,8 +119,8 @@ impl InterruptMethod { } } -pub struct MsixCfg { - pub cap: MsixCapability, +pub struct MappedMsixRegs { + pub info: MsixInfo, pub table: &'static mut [MsixTableEntry], } diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index 7b95c4fe84..e1b25fbbb5 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -4,7 +4,7 @@ use pcid_interface::irq_helpers::{allocate_single_interrupt_vector_for_msi, read use pcid_interface::msi::MsixTableEntry; use std::{fs::File, ptr::NonNull}; -use crate::{probe::MsixInfo, MSIX_PRIMARY_VECTOR}; +use crate::{probe::MappedMsixRegs, MSIX_PRIMARY_VECTOR}; use pcid_interface::*; @@ -12,19 +12,19 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { let pci_config = pcid_handle.fetch_config()?; // Extended message signaled interrupts. - let capability = match pcid_handle.feature_info(PciFeature::MsiX)? { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX)? { PciFeatureInfo::MsiX(capability) => capability, _ => unreachable!(), }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - let bar = &pci_config.func.bars[capability.table_bir() as usize]; + let bar = &pci_config.func.bars[msix_info.table_bar as usize]; let bar_address = unsafe { bar.physmap_mem("virtio-core") } as usize; - let virt_table_base = (bar_address + capability.table_offset() as usize) as *mut MsixTableEntry; + let virt_table_base = (bar_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - let mut info = MsixInfo { + let mut info = MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate the primary MSI vector. diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index e61898f848..7650736993 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -2,7 +2,7 @@ use std::fs::File; use std::ptr::NonNull; use std::sync::Arc; -use pcid_interface::msi::{MsixCapability, MsixTableEntry}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::*; use crate::spec::*; @@ -20,18 +20,18 @@ pub struct Device { unsafe impl Send for Device {} unsafe impl Sync for Device {} -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl MsixInfo { +impl MappedMsixRegs { pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { &mut *self.virt_table_base.as_ptr().add(k) } pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 047e16e148..ef5b122beb 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -119,18 +119,18 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, bar0_address: usize) -> (O (Some(interrupt_handle), InterruptMethod::Msi) } else if has_msix { - let capability = match pcid_handle.feature_info(PciFeature::MsiX).expect("xhcid: failed to retrieve the MSI-X capability structure from pcid") { + let msix_info = match pcid_handle.feature_info(PciFeature::MsiX).expect("xhcid: failed to retrieve the MSI-X capability structure from pcid") { PciFeatureInfo::Msi(_) => panic!(), PciFeatureInfo::MsiX(s) => s, }; - capability.validate(pci_config.func.bars); + msix_info.validate(pci_config.func.bars); - assert_eq!(capability.table_bir(), 0); - let virt_table_base = (bar0_address + capability.table_offset() as usize) as *mut MsixTableEntry; + assert_eq!(msix_info.table_bar, 0); + let virt_table_base = (bar0_address + msix_info.table_offset as usize) as *mut MsixTableEntry; - let mut info = xhci::MsixInfo { + let mut info = xhci::MappedMsixRegs { virt_table_base: NonNull::new(virt_table_base).unwrap(), - capability, + info: msix_info, }; // Allocate one msi vector. diff --git a/xhcid/src/xhci/mod.rs b/xhcid/src/xhci/mod.rs index d88a87c7be..32eb7f7dd4 100644 --- a/xhcid/src/xhci/mod.rs +++ b/xhcid/src/xhci/mod.rs @@ -21,7 +21,7 @@ use serde::Deserialize; use crate::usb; -use pcid_interface::msi::{MsixTableEntry, MsixCapability}; +use pcid_interface::msi::{MsixInfo, MsixTableEntry}; use pcid_interface::{PcidServerHandle, PciFeature}; mod capability; @@ -64,19 +64,19 @@ pub enum InterruptMethod { Msi, /// Extended message signaled interrupts. - MsiX(Mutex), + MsiX(Mutex), } -pub struct MsixInfo { +pub struct MappedMsixRegs { pub virt_table_base: NonNull, - pub capability: MsixCapability, + pub info: MsixInfo, } -impl MsixInfo { +impl MappedMsixRegs { pub unsafe fn table_entry_pointer_unchecked(&mut self, k: usize) -> &mut MsixTableEntry { &mut *self.virt_table_base.as_ptr().offset(k as isize) } pub fn table_entry_pointer(&mut self, k: usize) -> &mut MsixTableEntry { - assert!(k < self.capability.table_size() as usize); + assert!(k < self.info.table_size as usize); unsafe { self.table_entry_pointer_unchecked(k) } } } @@ -774,13 +774,13 @@ impl Xhci { if let InterruptMethod::MsiX(_) = self.interrupt_method { true } else { false } } // TODO: Perhaps use an rwlock? - pub fn msix_info(&self) -> Option> { + pub fn msix_info(&self) -> Option> { match self.interrupt_method { InterruptMethod::MsiX(ref info) => Some(info.lock().unwrap()), _ => None, } } - pub fn msix_info_mut(&self) -> Option> { + pub fn msix_info_mut(&self) -> Option> { match self.interrupt_method { InterruptMethod::MsiX(ref info) => Some(info.lock().unwrap()), _ => None,