Merge changes from topic "avf-b-support" into main
* changes:
pvmfw: fix device_info_assigned_info_without_iommus test
pvmfw: Handle absence of vcpu-stall-detector to avoid boot failure
libhypervisor_backends: gunyah: Implement device assigner
pvmfw: Do not return error when interrupts property is not present
pvmfw: Do iommus validation for pkvm-pviommu only
diff --git a/guest/pvmfw/src/device_assignment.rs b/guest/pvmfw/src/device_assignment.rs
index fb485fe..ba13fa3 100644
--- a/guest/pvmfw/src/device_assignment.rs
+++ b/guest/pvmfw/src/device_assignment.rs
@@ -373,10 +373,12 @@
// see: DeviceAssignmentInfo::validate_all_regs().
let mut all_iommus = BTreeSet::new();
for physical_device in physical_devices.values() {
- for iommu in &physical_device.iommus {
- if !all_iommus.insert(iommu) {
- error!("Unsupported phys IOMMU duplication found, <iommus> = {iommu:?}");
- return Err(DeviceAssignmentError::UnsupportedIommusDuplication);
+ if let Some(iommus) = &physical_device.iommus {
+ for iommu in iommus {
+ if !all_iommus.insert(iommu) {
+ error!("Unsupported phys IOMMU duplication found, <iommus> = {iommu:?}");
+ return Err(DeviceAssignmentError::UnsupportedIommusDuplication);
+ }
}
}
}
@@ -692,17 +694,17 @@
struct PhysicalDeviceInfo {
target: Phandle,
reg: Vec<DeviceReg>,
- iommus: Vec<(PhysIommu, Sid)>,
+ iommus: Option<Vec<(PhysIommu, Sid)>>,
}
impl PhysicalDeviceInfo {
fn parse_iommus(
node: &FdtNode,
phys_iommus: &BTreeMap<Phandle, PhysIommu>,
- ) -> Result<Vec<(PhysIommu, Sid)>> {
+ ) -> Result<Option<Vec<(PhysIommu, Sid)>>> {
let mut iommus = vec![];
let Some(mut cells) = node.getprop_cells(c"iommus")? else {
- return Ok(iommus);
+ return Ok(None);
};
while let Some(cell) = cells.next() {
// Parse pIOMMU ID
@@ -717,7 +719,7 @@
iommus.push((*iommu, Sid::from(cell)));
}
- Ok(iommus)
+ Ok(Some(iommus))
}
fn parse(node: &FdtNode, phys_iommus: &BTreeMap<Phandle, PhysIommu>) -> Result<Option<Self>> {
@@ -740,9 +742,9 @@
// <reg> property from the crosvm DT
reg: Vec<DeviceReg>,
// <interrupts> property from the crosvm DT
- interrupts: Vec<u8>,
+ interrupts: Option<Vec<u8>>,
// Parsed <iommus> property from the crosvm DT. Tuple of PvIommu and vSID.
- iommus: Vec<(PvIommu, Vsid)>,
+ iommus: Option<Vec<(PvIommu, Vsid)>>,
}
impl AssignedDeviceInfo {
@@ -794,19 +796,18 @@
Ok(())
}
- fn parse_interrupts(node: &FdtNode) -> Result<Vec<u8>> {
+ fn parse_interrupts(node: &FdtNode) -> Result<Option<Vec<u8>>> {
+ let Some(cells) = node.getprop_cells(c"interrupts")? else {
+ return Ok(None);
+ };
// Validation: Validate if interrupts cell numbers are multiple of #interrupt-cells.
// We can't know how many interrupts would exist.
- let interrupts_cells = node
- .getprop_cells(c"interrupts")?
- .ok_or(DeviceAssignmentError::InvalidInterrupts)?
- .count();
- if interrupts_cells % CELLS_PER_INTERRUPT != 0 {
+ if cells.count() % CELLS_PER_INTERRUPT != 0 {
return Err(DeviceAssignmentError::InvalidInterrupts);
}
// Once validated, keep the raw bytes so patch can be done with setprop()
- Ok(node.getprop(c"interrupts").unwrap().unwrap().into())
+ Ok(Some(node.getprop(c"interrupts").unwrap().unwrap().into()))
}
// TODO(b/277993056): Also validate /__local_fixups__ to ensure that <iommus> has phandle.
@@ -895,8 +896,16 @@
let interrupts = Self::parse_interrupts(&node)?;
- let iommus = Self::parse_iommus(&node, pviommus)?;
- Self::validate_iommus(&iommus, &physical_device.iommus, hypervisor)?;
+ // Ignore <iommus> if no pvIOMMUs are expected based on the VM DTBO, possibly
+ // because physical IOMMUs are being assigned directly.
+ let iommus = if let Some(iommus) = &physical_device.iommus {
+ let parsed_iommus = Self::parse_iommus(&node, pviommus)?;
+ Self::validate_iommus(&parsed_iommus, iommus, hypervisor)?;
+ Some(parsed_iommus)
+ } else {
+ // TODO: Detect misconfigured iommus in input DT.
+ None
+ };
Ok(Some(Self { node_path, reg, interrupts, iommus }))
}
@@ -904,14 +913,21 @@
fn patch(&self, fdt: &mut Fdt, pviommu_phandles: &BTreeMap<PvIommu, Phandle>) -> Result<()> {
let mut dst = fdt.node_mut(&self.node_path)?.unwrap();
dst.setprop(c"reg", &to_be_bytes(&self.reg))?;
- dst.setprop(c"interrupts", &self.interrupts)?;
- let mut iommus = Vec::with_capacity(8 * self.iommus.len());
- for (pviommu, vsid) in &self.iommus {
- let phandle = pviommu_phandles.get(pviommu).unwrap();
- iommus.extend_from_slice(&u32::from(*phandle).to_be_bytes());
- iommus.extend_from_slice(&vsid.0.to_be_bytes());
+ if let Some(interrupts) = &self.interrupts {
+ dst.setprop(c"interrupts", interrupts)?;
+ } else {
+ dst.nop_property(c"interrupts")?;
}
- dst.setprop(c"iommus", &iommus)?;
+
+ if let Some(iommus) = &self.iommus {
+ let mut iommus_vec = Vec::with_capacity(8 * iommus.len());
+ for (pviommu, vsid) in iommus {
+ let phandle = pviommu_phandles.get(pviommu).unwrap();
+ iommus_vec.extend_from_slice(&u32::from(*phandle).to_be_bytes());
+ iommus_vec.extend_from_slice(&vsid.0.to_be_bytes());
+ }
+ dst.setprop(c"iommus", &iommus_vec)?;
+ }
Ok(())
}
@@ -946,10 +962,12 @@
fn validate_pviommu_topology(assigned_devices: &[AssignedDeviceInfo]) -> Result<()> {
let mut all_iommus = BTreeSet::new();
for assigned_device in assigned_devices {
- for iommu in &assigned_device.iommus {
- if !all_iommus.insert(iommu) {
- error!("Unsupported pvIOMMU duplication found, <iommus> = {iommu:?}");
- return Err(DeviceAssignmentError::UnsupportedPvIommusDuplication);
+ if let Some(iommus) = &assigned_device.iommus {
+ for iommu in iommus {
+ if !all_iommus.insert(iommu) {
+ error!("Unsupported pvIOMMU duplication found, <iommus> = {iommu:?}");
+ return Err(DeviceAssignmentError::UnsupportedPvIommusDuplication);
+ }
}
}
}
@@ -1282,6 +1300,8 @@
// TODO(ptosi): Add tests with varying HYP_GRANULE values.
+ // TODO(ptosi): Add tests with iommus.is_none()
+
#[test]
fn device_info_new_without_symbols() {
let mut fdt_data = fs::read(FDT_FILE_PATH).unwrap();
@@ -1328,8 +1348,8 @@
let expected = [AssignedDeviceInfo {
node_path: CString::new("/bus0/backlight").unwrap(),
reg: vec![[0x9, 0xFF].into()],
- interrupts: into_fdt_prop(vec![0x0, 0xF, 0x4]),
- iommus: vec![],
+ interrupts: Some(into_fdt_prop(vec![0x0, 0xF, 0x4])),
+ iommus: None,
}];
assert_eq!(device_info.assigned_devices, expected);
@@ -1353,8 +1373,8 @@
let expected = [AssignedDeviceInfo {
node_path: CString::new("/rng").unwrap(),
reg: vec![[0x9, 0xFF].into()],
- interrupts: into_fdt_prop(vec![0x0, 0xF, 0x4]),
- iommus: vec![(PvIommu { id: 0x4 }, Vsid(0xFF0))],
+ interrupts: Some(into_fdt_prop(vec![0x0, 0xF, 0x4])),
+ iommus: Some(vec![(PvIommu { id: 0x4 }, Vsid(0xFF0))]),
}];
assert_eq!(device_info.assigned_devices, expected);
@@ -1435,7 +1455,6 @@
(Ok(c"android,backlight,ignore-gctrl-reset"), Ok(Vec::new())),
(Ok(c"compatible"), Ok(Vec::from(*b"android,backlight\0"))),
(Ok(c"interrupts"), Ok(into_fdt_prop(vec![0x0, 0xF, 0x4]))),
- (Ok(c"iommus"), Ok(Vec::new())),
(Ok(c"phandle"), Ok(into_fdt_prop(vec![phandle.unwrap()]))),
(Ok(c"reg"), Ok(into_fdt_prop(vec![0x0, 0x9, 0x0, 0xFF]))),
];
diff --git a/guest/pvmfw/src/fdt.rs b/guest/pvmfw/src/fdt.rs
index 2444b00..b5d3b28 100644
--- a/guest/pvmfw/src/fdt.rs
+++ b/guest/pvmfw/src/fdt.rs
@@ -874,9 +874,13 @@
}
}
-fn read_wdt_info_from(fdt: &Fdt) -> libfdt::Result<WdtInfo> {
+fn read_wdt_info_from(fdt: &Fdt) -> libfdt::Result<Option<WdtInfo>> {
let mut node_iter = fdt.compatible_nodes(c"qemu,vcpu-stall-detector")?;
- let node = node_iter.next().ok_or(FdtError::NotFound)?;
+ let Some(node) = node_iter.next() else {
+ // Some VMs may not have qemu,vcpu-stall-detector compatible watchdogs.
+ // Do not treat it as error.
+ return Ok(None);
+ };
let mut ranges = node.reg()?.ok_or(FdtError::NotFound)?;
let reg = ranges.next().ok_or(FdtError::NotFound)?;
@@ -893,7 +897,7 @@
warn!("Discarding extra vmwdt <interrupts> entries.");
}
- Ok(WdtInfo { addr: reg.addr, size, irq })
+ Ok(Some(WdtInfo { addr: reg.addr, size, irq }))
}
fn validate_wdt_info(wdt: &WdtInfo, num_cpus: usize) -> Result<(), RebootReason> {
@@ -905,18 +909,25 @@
Ok(())
}
-fn patch_wdt_info(fdt: &mut Fdt, num_cpus: usize) -> libfdt::Result<()> {
- let mut interrupts = WdtInfo::get_expected(num_cpus).irq;
- for v in interrupts.iter_mut() {
- *v = v.to_be();
- }
-
+fn patch_wdt_info(
+ fdt: &mut Fdt,
+ wdt_info: &Option<WdtInfo>,
+ num_cpus: usize,
+) -> libfdt::Result<()> {
let mut node = fdt
.root_mut()
.next_compatible(c"qemu,vcpu-stall-detector")?
.ok_or(libfdt::FdtError::NotFound)?;
- node.setprop_inplace(c"interrupts", interrupts.as_bytes())?;
- Ok(())
+
+ if wdt_info.is_some() {
+ let mut interrupts = WdtInfo::get_expected(num_cpus).irq;
+ for v in interrupts.iter_mut() {
+ *v = v.to_be();
+ }
+ node.setprop_inplace(c"interrupts", interrupts.as_bytes())
+ } else {
+ node.nop()
+ }
}
/// Patch the DT by deleting the ns16550a compatible nodes whose address are unknown
@@ -1088,6 +1099,7 @@
untrusted_props: BTreeMap<CString, Vec<u8>>,
vm_ref_dt_props_info: BTreeMap<CString, Vec<u8>>,
vcpufreq_info: Option<VcpufreqInfo>,
+ wdt_info: Option<WdtInfo>,
}
impl DeviceTreeInfo {
@@ -1218,7 +1230,9 @@
error!("Failed to read vCPU stall detector info from DT: {e}");
RebootReason::InvalidFdt
})?;
- validate_wdt_info(&wdt_info, cpus.len())?;
+ if let Some(ref info) = wdt_info {
+ validate_wdt_info(info, cpus.len())?;
+ }
let serial_info = read_serial_info_from(fdt).map_err(|e| {
error!("Failed to read serial info from DT: {e}");
@@ -1284,6 +1298,7 @@
untrusted_props,
vm_ref_dt_props_info,
vcpufreq_info,
+ wdt_info,
})
}
@@ -1316,7 +1331,7 @@
error!("Failed to patch pci info to DT: {e}");
RebootReason::InvalidFdt
})?;
- patch_wdt_info(fdt, info.cpus.len()).map_err(|e| {
+ patch_wdt_info(fdt, &info.wdt_info, info.cpus.len()).map_err(|e| {
error!("Failed to patch wdt info to DT: {e}");
RebootReason::InvalidFdt
})?;
diff --git a/guest/pvmfw/testdata/expected_dt_with_dependency.dts b/guest/pvmfw/testdata/expected_dt_with_dependency.dts
index 7e0ad20..2823e1a 100644
--- a/guest/pvmfw/testdata/expected_dt_with_dependency.dts
+++ b/guest/pvmfw/testdata/expected_dt_with_dependency.dts
@@ -11,7 +11,6 @@
dep = <&node_a_dep &common>;
reg = <0x0 0xFF000 0x0 0x1>;
interrupts = <0x0 0xF 0x4>;
- iommus;
};
node_a_dep: node_a_dep {
diff --git a/guest/pvmfw/testdata/expected_dt_with_dependency_loop.dts b/guest/pvmfw/testdata/expected_dt_with_dependency_loop.dts
index 61031ab..b1ce262 100644
--- a/guest/pvmfw/testdata/expected_dt_with_dependency_loop.dts
+++ b/guest/pvmfw/testdata/expected_dt_with_dependency_loop.dts
@@ -8,7 +8,6 @@
loop_dep = <&node_c_loop>;
reg = <0x0 0xFF200 0x0 0x1>;
interrupts = <0x0 0xF 0x4>;
- iommus;
};
node_c_loop: node_c_loop {
diff --git a/guest/pvmfw/testdata/expected_dt_with_multiple_dependencies.dts b/guest/pvmfw/testdata/expected_dt_with_multiple_dependencies.dts
index dc8c357..d31c8cd 100644
--- a/guest/pvmfw/testdata/expected_dt_with_multiple_dependencies.dts
+++ b/guest/pvmfw/testdata/expected_dt_with_multiple_dependencies.dts
@@ -13,7 +13,6 @@
dep = <&node_a_dep &common>;
reg = <0x0 0xFF000 0x0 0x1>;
interrupts = <0x0 0xF 0x4>;
- iommus;
};
node_a_dep: node_a_dep {
@@ -38,7 +37,6 @@
dep = <&node_b_dep1 &node_b_dep2>;
reg = <0x00 0xFF100 0x00 0x01>;
interrupts = <0x00 0x0F 0x04>;
- iommus;
};
node_b_dep1: node_b_dep1 {
diff --git a/libs/libhypervisor_backends/src/error.rs b/libs/libhypervisor_backends/src/error.rs
index 3046b0c..ffc6c57 100644
--- a/libs/libhypervisor_backends/src/error.rs
+++ b/libs/libhypervisor_backends/src/error.rs
@@ -18,6 +18,8 @@
#[cfg(target_arch = "aarch64")]
use super::hypervisor::GeniezoneError;
+#[cfg(target_arch = "aarch64")]
+use super::hypervisor::GunyahError;
use super::hypervisor::KvmError;
#[cfg(target_arch = "aarch64")]
use uuid::Uuid;
@@ -41,6 +43,9 @@
#[cfg(target_arch = "x86_64")]
/// Unsupported x86_64 Hypervisor
UnsupportedHypervisor(u128),
+ #[cfg(target_arch = "aarch64")]
+ /// Failed to invoke Gunyah HVC.
+ GunyahError(GunyahError),
}
impl fmt::Display for Error {
@@ -58,6 +63,10 @@
)
}
#[cfg(target_arch = "aarch64")]
+ Self::GunyahError(e) => {
+ write!(f, "Failed to invoke Gunyah HVC: {e}")
+ }
+ #[cfg(target_arch = "aarch64")]
Self::UnsupportedHypervisorUuid(u) => {
write!(f, "Unsupported Hypervisor UUID {u}")
}
diff --git a/libs/libhypervisor_backends/src/hypervisor.rs b/libs/libhypervisor_backends/src/hypervisor.rs
index 7c274f5..81008f1 100644
--- a/libs/libhypervisor_backends/src/hypervisor.rs
+++ b/libs/libhypervisor_backends/src/hypervisor.rs
@@ -39,6 +39,9 @@
#[cfg(target_arch = "aarch64")]
pub use geniezone::GeniezoneError;
+#[cfg(target_arch = "aarch64")]
+pub use gunyah::GunyahError;
+
use alloc::boxed::Box;
use common::Hypervisor;
pub use common::{DeviceAssigningHypervisor, MemSharingHypervisor, MmioGuardedHypervisor};
diff --git a/libs/libhypervisor_backends/src/hypervisor/gunyah.rs b/libs/libhypervisor_backends/src/hypervisor/gunyah.rs
index 45c01bf..f25d15a 100644
--- a/libs/libhypervisor_backends/src/hypervisor/gunyah.rs
+++ b/libs/libhypervisor_backends/src/hypervisor/gunyah.rs
@@ -1,10 +1,42 @@
use super::common::Hypervisor;
+use super::DeviceAssigningHypervisor;
+use crate::{Error, Result};
+use thiserror::Error;
use uuid::{uuid, Uuid};
+const SIZE_4KB: usize = 4 << 10;
+
pub(super) struct GunyahHypervisor;
+/// Error from a Gunyah HVC call.
+#[derive(Copy, Clone, Debug, Eq, Error, PartialEq)]
+pub enum GunyahError {
+ /// The call is not supported by the implementation.
+ #[error("Gunyah call not supported")]
+ NotSupported,
+}
+
impl GunyahHypervisor {
pub const UUID: Uuid = uuid!("c1d58fcd-a453-5fdb-9265-ce36673d5f14");
}
-impl Hypervisor for GunyahHypervisor {}
+impl Hypervisor for GunyahHypervisor {
+ fn as_device_assigner(&self) -> Option<&dyn DeviceAssigningHypervisor> {
+ Some(self)
+ }
+
+ fn get_granule_size(&self) -> Option<usize> {
+ Some(SIZE_4KB)
+ }
+}
+
+impl DeviceAssigningHypervisor for GunyahHypervisor {
+ fn get_phys_mmio_token(&self, base_ipa: u64) -> Result<u64> {
+ // PA = IPA for now.
+ Ok(base_ipa)
+ }
+
+ fn get_phys_iommu_token(&self, _pviommu_id: u64, _vsid: u64) -> Result<(u64, u64)> {
+ Err(Error::GunyahError(GunyahError::NotSupported))
+ }
+}