From b779c5cdef951b02db7b0e2fda31f3dd2c1d020b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre-Cl=C3=A9ment=20Tosi?= Date: Wed, 24 Aug 2022 23:21:10 +0100 Subject: [PATCH] hypervisor: ProtectionType: Add helper methods Introduce a set of helper methods that expose individual characteristics that particular subsets of the ProtectionType variants share. These will simplify various match statements into conditionals by encapsulating the deduction of those characteristics, making the calling code more concise and robust. BUG=b:243646855 TEST=build Change-Id: I65ff3e61c448d90704551b79c21c64d1b4c11dc4 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3944852 Reviewed-by: Keiichi Watanabe --- aarch64/src/lib.rs | 61 ++++++++++++++--------------------- hypervisor/src/kvm/aarch64.rs | 9 +++--- hypervisor/src/lib.rs | 17 ++++++++++ src/crosvm/sys/unix.rs | 6 +--- 4 files changed, 47 insertions(+), 46 deletions(-) diff --git a/aarch64/src/lib.rs b/aarch64/src/lib.rs index bcf5a7f0b6..e09c3ab392 100644 --- a/aarch64/src/lib.rs +++ b/aarch64/src/lib.rs @@ -278,10 +278,7 @@ impl arch::LinuxArch for AArch64 { vec![(GuestAddress(AARCH64_PHYS_MEM_START), components.memory_size)]; // Allocate memory for the pVM firmware. - if matches!( - components.hv_cfg.protection_type, - ProtectionType::Protected | ProtectionType::UnprotectedWithFirmware - ) { + if components.hv_cfg.protection_type.runs_firmware() { memory_regions.push(( GuestAddress(AARCH64_PROTECTED_VM_FW_START), AARCH64_PROTECTED_VM_FW_MAX_SIZE, @@ -401,28 +398,23 @@ impl arch::LinuxArch for AArch64 { .map_err(Error::MapPvtimeError)?; } - match components.hv_cfg.protection_type { - ProtectionType::Protected => { - // Tell the hypervisor to load the pVM firmware. - vm.load_protected_vm_firmware( - GuestAddress(AARCH64_PROTECTED_VM_FW_START), - AARCH64_PROTECTED_VM_FW_MAX_SIZE, - ) - .map_err(Error::ProtectVm)?; - } - ProtectionType::UnprotectedWithFirmware => { - // Load pVM firmware ourself, as the VM is not really protected. - // `components.pvm_fw` is safe to unwrap because `protection_type` is - // `UnprotectedWithFirmware`. - arch::load_image( - &mem, - &mut components.pvm_fw.unwrap(), - GuestAddress(AARCH64_PROTECTED_VM_FW_START), - AARCH64_PROTECTED_VM_FW_MAX_SIZE, - ) - .map_err(Error::PvmFwLoadFailure)?; - } - ProtectionType::Unprotected | ProtectionType::ProtectedWithoutFirmware => {} + if components.hv_cfg.protection_type.loads_firmware() { + arch::load_image( + &mem, + &mut components + .pvm_fw + .expect("pvmfw must be available if ProtectionType loads it"), + GuestAddress(AARCH64_PROTECTED_VM_FW_START), + AARCH64_PROTECTED_VM_FW_MAX_SIZE, + ) + .map_err(Error::PvmFwLoadFailure)?; + } else if components.hv_cfg.protection_type.runs_firmware() { + // Tell the hypervisor to load the pVM firmware. + vm.load_protected_vm_firmware( + GuestAddress(AARCH64_PROTECTED_VM_FW_START), + AARCH64_PROTECTED_VM_FW_MAX_SIZE, + ) + .map_err(Error::ProtectVm)?; } for (vcpu_id, vcpu) in vcpus.iter().enumerate() { @@ -858,12 +850,12 @@ impl AArch64 { get_kernel_addr() }; - let entry_addr = match protection_type { - ProtectionType::Protected => None, // Hypervisor controls the entry point - ProtectionType::UnprotectedWithFirmware => Some(AARCH64_PROTECTED_VM_FW_START), - ProtectionType::Unprotected | ProtectionType::ProtectedWithoutFirmware => { - Some(image_addr.offset()) - } + let entry_addr = if protection_type.loads_firmware() { + Some(AARCH64_PROTECTED_VM_FW_START) + } else if protection_type.runs_firmware() { + None // Initial PC value is set by the hypervisor + } else { + Some(image_addr.offset()) }; /* PC -- entry point */ @@ -878,10 +870,7 @@ impl AArch64 { vcpu.set_one_reg(VcpuRegAArch64::X(0), fdt_addr.offset()) .map_err(Error::SetReg)?; - if matches!( - protection_type, - ProtectionType::Protected | ProtectionType::UnprotectedWithFirmware - ) { + if protection_type.runs_firmware() { /* X1 -- payload entry point */ vcpu.set_one_reg(VcpuRegAArch64::X(1), image_addr.offset()) .map_err(Error::SetReg)?; diff --git a/hypervisor/src/kvm/aarch64.rs b/hypervisor/src/kvm/aarch64.rs index 6e98647474..7718dfa922 100644 --- a/hypervisor/src/kvm/aarch64.rs +++ b/hypervisor/src/kvm/aarch64.rs @@ -66,11 +66,10 @@ impl Kvm { ret if ret < 0 => 0, ipa => ipa as u32, }; - let protection_flag = match protection_type { - ProtectionType::Unprotected | ProtectionType::UnprotectedWithFirmware => 0, - ProtectionType::Protected | ProtectionType::ProtectedWithoutFirmware => { - KVM_VM_TYPE_ARM_PROTECTED - } + let protection_flag = if protection_type.isolates_memory() { + KVM_VM_TYPE_ARM_PROTECTED + } else { + 0 }; // Use the lower 8 bits representing the IPA space as the machine type Ok((ipa_size & KVM_VM_TYPE_ARM_IPA_SIZE_MASK) | protection_flag) diff --git a/hypervisor/src/lib.rs b/hypervisor/src/lib.rs index e494c0e245..9a29b74d4e 100644 --- a/hypervisor/src/lib.rs +++ b/hypervisor/src/lib.rs @@ -559,6 +559,23 @@ pub enum ProtectionType { UnprotectedWithFirmware, } +impl ProtectionType { + /// Returns whether the hypervisor will prevent us from accessing the VM's memory. + pub fn isolates_memory(&self) -> bool { + matches!(self, Self::Protected | Self::ProtectedWithoutFirmware) + } + + /// Returns whether the VMM needs to load the pVM firmware. + pub fn loads_firmware(&self) -> bool { + matches!(self, Self::UnprotectedWithFirmware) + } + + /// Returns whether the VM runs a pVM firmware. + pub fn runs_firmware(&self) -> bool { + self.loads_firmware() || matches!(self, Self::Protected) + } +} + #[derive(Clone, Copy)] pub struct Config { #[cfg(target_arch = "aarch64")] diff --git a/src/crosvm/sys/unix.rs b/src/crosvm/sys/unix.rs index 6116ee4d9b..0ac69f3f90 100644 --- a/src/crosvm/sys/unix.rs +++ b/src/crosvm/sys/unix.rs @@ -1282,11 +1282,7 @@ fn run_kvm(cfg: Config, components: VmComponents, guest_mem: GuestMemory) -> Res } // Check that the VM was actually created in protected mode as expected. - if matches!( - cfg.protection_type, - ProtectionType::Protected | ProtectionType::ProtectedWithoutFirmware - ) && !vm.check_capability(VmCap::Protected) - { + if cfg.protection_type.isolates_memory() && !vm.check_capability(VmCap::Protected) { bail!("Failed to create protected VM"); } let vm_clone = vm.try_clone().context("failed to clone vm")?;