Performance optimizations (#95)

* fixed bug when running multi vCPU CVMs, optimized IPI code and context switch
* improve performance by reducing cost of operation in critical section
* add fine grained locking mechanism
* optimize access to memory protector

---------

Signed-off-by: Wojciech Ozga <[email protected]>

Author: wojciechozga

confidential-vms/linux_vm/hypervisor_rootfs/run_linux_vm_qemu2.sh

@@ -0,0 +1,69 @@
+#!/usr/bin/env bash
+# SPDX-FileCopyrightText: 2023 IBM Corporation
+# SPDX-FileContributor: Wojciech Ozga <[email protected]>, IBM Research - Zurich
+# SPDX-License-Identifier: Apache-2.0
+
+if [ ! -f "/root/linux_vm2" ]; then
+  cp -rf /root/linux_vm /root/linux_vm2
+fi
+
+QEMU_CMD=qemu-system-riscv64
+KERNEL=/root/linux_vm2/Image
+DRIVE=/root/linux_vm2/rootfs.ext2
+INITRAMFS=/root/linux_vm2/rootfs.cpio
+TAP=/root/linux_vm2/cove_tap_qemu
+
+HOST_PORT="$((3000 + RANDOM % 3000))"
+INTERACTIVE="-nographic"
+SMP=2
+MEMORY=1G
+
+for i in "$@"; do
+  case $i in
+    -e=*|--debug-port=*)
+      DEBUG_PORT="${i#*=}"
+      DEBUG_OPTIONS="-gdb tcp::${DEBUG_PORT} -S -d in_asm -D debug.log"
+      echo ${DEBUG_OPTIONS}
+      shift
+      ;;
+    --host-port=*)
+      HOST_PORT="${i#*=}"
+      shift
+      ;;
+    -s=*|--smp=*)
+      SMP="${i#*=}"
+      shift
+      ;;
+    -m=*|--memory=*)
+      MEMORY="${i#*=}"
+      shift
+      ;;
+    --daemonize*)
+      INTERACTIVE="-daemonize"
+      shift
+      ;;
+    -*|--*)
+      echo "Unknown option $i"
+      exit 1
+      ;;
+    *)
+      ;;
+  esac
+done
+
+echo "SSH port: ${HOST_PORT}"
+echo "Number of cores assigned to the guest: ${SMP}"
+
+${QEMU_CMD} ${DEBUG_OPTIONS} \
+    ${INTERACTIVE} \
+    --enable-kvm \
+    -machine virt,cove=true,cove-tap-filename=${TAP} -cpu rv64,f=true -smp ${SMP} -m ${MEMORY} \
+    -kernel ${KERNEL} \
+    -seed 0 \
+    -global virtio-mmio.force-legacy=false \
+    -append "console=ttyS0 ro root=/dev/vda swiotlb=mmnn,force" \
+    -device virtio-blk-pci,drive=hd0,iommu_platform=on,disable-legacy=on,disable-modern=off \
+    -drive if=none,format=raw,file=${DRIVE},id=hd0 \
+    -device virtio-net-pci,netdev=net0,iommu_platform=on,disable-legacy=on,disable-modern=off \
+    -netdev user,id=net0,net=192.168.100.1/24,dhcpstart=192.168.100.128,hostfwd=tcp::${HOST_PORT}-:22 \
+    -nographic

security-monitor/src/confidential_flow/finite_state_machine.rs

@@ -31,6 +31,7 @@ use crate::core::control_data::{
     ConfidentialHart, ConfidentialHartRemoteCommand, ConfidentialVm, ConfidentialVmId, ControlDataStorage, HardwareHart, HypervisorHart,
     ResumableOperation,
 };
+use crate::core::interrupt_controller::InterruptController;
 use crate::error::Error;
 use crate::non_confidential_flow::{DeclassifyToHypervisor, NonConfidentialFlow};
 
@@ -118,11 +119,18 @@ impl<'a> ConfidentialFlow<'a> {
         hardware_hart: &'a mut HardwareHart, confidential_vm_id: ConfidentialVmId, confidential_hart_id: usize,
     ) -> Result<(usize, Self), (&'a mut HardwareHart, Error)> {
         assert!(hardware_hart.confidential_hart().is_dummy());
-        match ControlDataStorage::try_confidential_vm(confidential_vm_id, |mut confidential_vm| {
+        // Now, we are going to change the context between security domains.
+        // 1) Store the hypervisor hart state that executed on this physical hart to the main memory.
+        hardware_hart.hypervisor_hart_mut().save_in_main_memory();
+        match ControlDataStorage::try_confidential_vm(confidential_vm_id, |confidential_vm| {
             confidential_vm.steal_confidential_hart(confidential_hart_id, hardware_hart)?;
             Ok(confidential_vm.allowed_external_interrupts())
         }) {
-            Ok(allowed_external_interrupts) => Ok((allowed_external_interrupts, Self { hardware_hart })),
+            Ok(allowed_external_interrupts) => {
+                // 2) Load confidential hart state from main memory to the physical hart executing this code.
+                hardware_hart.confidential_hart_mut().restore_from_main_memory();
+                Ok((allowed_external_interrupts, Self { hardware_hart }))
+            }
             Err(error) => Err((hardware_hart, error)),
         }
     }
@@ -134,14 +142,20 @@ impl<'a> ConfidentialFlow<'a> {
         let declassifier =
             DeclassifyToHypervisor::EnabledInterrupts(ExposeEnabledInterrupts::from_confidential_hart(self.confidential_hart()));
 
-        ControlDataStorage::try_confidential_vm(self.confidential_vm_id(), |mut confidential_vm| {
-            // Run heavy context switch when giving back the confidential hart to the confidential VM.
-            confidential_vm.return_confidential_hart(self.hardware_hart);
-            Ok(NonConfidentialFlow::create(self.hardware_hart).declassify_to_hypervisor_hart(declassifier))
+        // Now, we are going to change the context between security domains.
+        // 1) Store the confidential hart state that executed on this physical hart to the main memory.
+        self.hardware_hart.confidential_hart_mut().save_in_main_memory();
+        let _ = ControlDataStorage::try_confidential_vm(self.confidential_vm_id(), |confidential_vm| {
+            Ok(confidential_vm.return_confidential_hart(self.hardware_hart))
         })
         // Below unwrap is safe because we are in the confidential flow that guarantees that the confidential VM with
         // the given id exists in the control data.
-        .unwrap()
+        .unwrap();
+        // Enable memory access control for the hypervisor
+        unsafe { self.hardware_hart.hypervisor_hart().enable_hypervisor_memory_protector() };
+        // 2) Load hypervisor hart state from main memory to the physical hart executing this code.
+        self.hardware_hart.hypervisor_hart_mut().restore_from_main_memory();
+        NonConfidentialFlow::create(self.hardware_hart).declassify_to_hypervisor_hart(declassifier)
     }
 
     /// Resumes execution of the confidential hart after the confidential hart was not running on any physical hart.
@@ -162,7 +176,7 @@ impl<'a> ConfidentialFlow<'a> {
         // load) to the hypervisor. We must handle the response or resume confidential hart's execution.
         use crate::core::control_data::ResumableOperation::*;
         match self.confidential_hart_mut().take_resumable_operation() {
-            Some(SbiRequest()) => SbiResponse::from_hypervisor_hart(self.hypervisor_hart()).handle(self),
+            Some(SbiRequest()) => SbiResponse::success().handle(self),
             Some(ResumeHart(v)) => v.handle(self),
             Some(MmioLoad(v)) => MmioLoadResponse::from_hypervisor_hart(self.hypervisor_hart(), v).handle(self),
             Some(MmioStore(v)) => MmioStoreResponse::from_hypervisor_hart(self.hypervisor_hart(), v).handle(self),
@@ -200,8 +214,7 @@ impl<'a> ConfidentialFlow<'a> {
         // We must restore the control and status registers (CSRs) that might have changed during execution of the security monitor.
         // We call it here because it is just before exiting to the assembly context switch, so we are sure that these CSRs have their
         // final values.
-        let interrupts =
-            self.confidential_hart().csrs().hvip.read_from_main_memory() | self.confidential_hart().csrs().vsip.read_from_main_memory();
+        let interrupts = self.confidential_hart().csrs().hvip.read_from_main_memory() | self.confidential_hart().csrs().pending_vssip_irqs;
         let address = self.confidential_hart_mut().address();
         self.confidential_hart().csrs().hvip.write(interrupts);
         self.confidential_hart().csrs().sscratch.write(address);
@@ -214,32 +227,44 @@ impl<'a> ConfidentialFlow<'a> {
     /// Broadcasts the inter hart request to confidential harts of the currently executing confidential VM. Returns error if sending an IPI
     /// to other confidential hart failed or if there is too many pending IPI queued.
     pub fn broadcast_remote_command(
-        &mut self, confidential_vm: &mut ConfidentialVm, confidential_hart_remote_command: ConfidentialHartRemoteCommand,
+        &mut self, confidential_vm: &ConfidentialVm, confidential_hart_remote_command: ConfidentialHartRemoteCommand,
     ) -> Result<(), Error> {
         let sender_confidential_hart_id = self.hardware_hart.confidential_hart().confidential_hart_id();
         // check if the remote command is also dedicated for the currently executing confidential hart
         if confidential_hart_remote_command.is_hart_selected(sender_confidential_hart_id) {
             self.hardware_hart.confidential_hart_mut().execute(&confidential_hart_remote_command);
         }
         // For the time-being, we rely on the OpenSBI's implementation of broadcasting IPIs to hardware harts.
-        self.hardware_hart
-            .opensbi_context(|| confidential_vm.broadcast_remote_command(sender_confidential_hart_id, confidential_hart_remote_command))
+        let hart_mask = confidential_vm.broadcast_remote_command(sender_confidential_hart_id, confidential_hart_remote_command)?;
+        if hart_mask != 0 {
+            // Confidential harts that should receive an ConfidentialHartRemoteCommand are currently running on a hardware
+            // harts. We interrupt such hardware harts with IPIs. Consequently, hardware harts running target confidential
+            // harts will trap into the security monitor, which will execute ConfidentialHartRemoteCommands on the target harts.
+            self.hardware_hart.opensbi_context(|| {
+                InterruptController::try_read(|controller| controller.send_ipis(hart_mask, 0))?;
+                Ok(())
+            })?;
+        }
+
+        Ok(())
     }
 
     /// Processes pending requests from other confidential harts by applying the corresponding state transformation to
     /// this confidential hart.
     ///
     /// This function must only be called when the hypervisor requested resume of confidential hart's execution or when
     /// a hardware hart executing a confidential hart is interrupted with the inter-processor-interrupt (IPI).
-    fn process_confidential_hart_remote_commands(&mut self) {
-        ControlDataStorage::try_confidential_vm(self.confidential_vm_id(), |mut confidential_vm| {
+    pub fn process_confidential_hart_remote_commands(&mut self) -> bool {
+        let mut requests_processed = false;
+        ControlDataStorage::try_confidential_vm(self.confidential_vm_id(), |confidential_vm| {
             confidential_vm.try_confidential_hart_remote_commands(
                 self.confidential_hart_id(),
                 |ref mut confidential_hart_remote_commands| {
                     confidential_hart_remote_commands.drain(..).for_each(|confidential_hart_remote_command| {
                         // The confidential flow has an ownership of the confidential hart because the confidential hart
                         // is assigned to the hardware hart.
                         self.confidential_hart_mut().execute(&confidential_hart_remote_command);
+                        requests_processed = true;
                     });
                     Ok(())
                 },
@@ -249,6 +274,7 @@ impl<'a> ConfidentialFlow<'a> {
         // confidential flow of the finite state machine (FSM) that guarantees it and 2) the processing of inter hart
         // requests always succeeds.
         .unwrap();
+        requests_processed
     }
 }
 

security-monitor/src/confidential_flow/handlers/attestation/retrieve_secret.rs

@@ -24,7 +24,7 @@ impl RetrieveSecretRequest {
     }
 
     pub fn handle(self, confidential_flow: ConfidentialFlow) -> ! {
-        let transformation = ControlDataStorage::try_confidential_vm(confidential_flow.confidential_vm_id(), |ref mut confidential_vm| {
+        let transformation = ControlDataStorage::try_confidential_vm(confidential_flow.confidential_vm_id(), |ref confidential_vm| {
             // ensure!(self.output_buffer_address.is_aligned_to(PageSize::Size4KiB.in_bytes()), Error::AddressNotAligned())?;
             ensure!(self.output_buffer_size <= PageSize::Size4KiB.in_bytes(), Error::AddressNotAligned())?;
             let secret = confidential_vm.secret(0)?;

security-monitor/src/confidential_flow/handlers/interrupts/allow_external_interrupt.rs

@@ -18,7 +18,7 @@ impl AllowExternalInterrupt {
     }
 
     pub fn handle(self, confidential_flow: ConfidentialFlow) -> ! {
-        match ControlDataStorage::try_confidential_vm(confidential_flow.confidential_vm_id(), |mut confidential_vm| {
+        match ControlDataStorage::try_confidential_vm_mut(confidential_flow.confidential_vm_id(), |mut confidential_vm| {
             Ok(confidential_vm.allow_external_interrupt(self.interrupt_id))
         }) {
             Ok(_) => confidential_flow

security-monitor/src/confidential_flow/handlers/interrupts/expose_enabled_interrupts.rs

@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: 2023 IBM Corporation
 // SPDX-FileContributor: Wojciech Ozga <[email protected]>, IBM Research - Zurich
 // SPDX-License-Identifier: Apache-2.0
-use crate::core::architecture::specification::{CSR_VSIE, CSR_VSTIMECMP};
+use crate::core::architecture::specification::{CSR_HVIP, CSR_VSIE, CSR_VSTIMECMP};
 use crate::core::control_data::{ConfidentialHart, HypervisorHart};
 
 pub struct ExposeEnabledInterrupts {
@@ -16,6 +16,7 @@ impl ExposeEnabledInterrupts {
 
     pub fn declassify_to_hypervisor_hart(&self, hypervisor_hart: &mut HypervisorHart) {
         hypervisor_hart.shared_memory_mut().write_csr(CSR_VSIE.into(), self.vsie);
+        hypervisor_hart.shared_memory_mut().write_csr(CSR_HVIP.into(), 0);
         hypervisor_hart.shared_memory_mut().write_csr(CSR_VSTIMECMP.into(), self.vstimecmp);
     }
 }

security-monitor/src/confidential_flow/handlers/interrupts/handle_interrupt.rs

@@ -3,37 +3,33 @@
 // SPDX-License-Identifier: Apache-2.0
 use crate::confidential_flow::handlers::sbi::SbiResponse;
 use crate::confidential_flow::ConfidentialFlow;
-use crate::core::architecture::specification::{MIE_SSIP_MASK, SCAUSE_INTERRUPT_MASK};
+use crate::core::architecture::specification::{MIE_MSIP_MASK, MIE_SSIP_MASK, SCAUSE_INTERRUPT_MASK};
+use crate::core::architecture::CSR;
 use crate::core::control_data::{ConfidentialHart, HypervisorHart};
 use crate::non_confidential_flow::DeclassifyToHypervisor;
 
 /// Handles an interrupt that occured during the execution of a confidential hart. The control flows (a) to the hypervisor when an interrupt
 /// comes from a hardware device, or (b) to the confidential hart in case of an IPI from other confidential hart.
-pub struct HandleInterrupt {
-    pending_interrupts: usize,
-}
+pub struct HandleInterrupt();
 
 impl HandleInterrupt {
-    pub fn from_confidential_hart(confidential_hart: &ConfidentialHart) -> Self {
-        Self { pending_interrupts: confidential_hart.csrs().mip.read() }
+    pub fn from_confidential_hart(_confidential_hart: &ConfidentialHart) -> Self {
+        Self {}
     }
 
-    pub fn handle(self, confidential_flow: ConfidentialFlow) -> ! {
-        if self.pending_interrupts & MIE_SSIP_MASK > 0 {
-            // One of the reasons why the confidential hart was interrupted with SSIP is that it got an `ConfidentialHartRemoteCommand` from
-            // another confidential hart. If this is the case, we must process all queued requests before resuming confidential
-            // hart's execution. This is done as part of the procedure that resumes confidential hart execution.
-            confidential_flow.resume_confidential_hart_execution();
-        } else {
-            // The only interrupts that we can see here are:
-            // * M-mode timer that the security monitor set to preemt execution of a confidential VM
-            // * M-mode software or external interrupt
-            confidential_flow.into_non_confidential_flow().declassify_and_exit_to_hypervisor(DeclassifyToHypervisor::Interrupt(self))
+    pub fn handle(self, mut confidential_flow: ConfidentialFlow) -> ! {
+        if CSR.mip.read() & MIE_MSIP_MASK > 0 {
+            if confidential_flow.process_confidential_hart_remote_commands() {
+                CSR.mip.read_and_clear_bits(MIE_SSIP_MASK);
+                confidential_flow.resume_confidential_hart_execution();
+            }
         }
+
+        confidential_flow.into_non_confidential_flow().declassify_and_exit_to_hypervisor(DeclassifyToHypervisor::Interrupt(self))
     }
 
     pub fn declassify_to_hypervisor_hart(&self, hypervisor_hart: &mut HypervisorHart) {
-        hypervisor_hart.csrs_mut().scause.write(self.pending_interrupts | SCAUSE_INTERRUPT_MASK);
+        hypervisor_hart.csrs_mut().scause.read_and_set_bits(SCAUSE_INTERRUPT_MASK);
         SbiResponse::success().declassify_to_hypervisor_hart(hypervisor_hart);
     }
 }

security-monitor/src/confidential_flow/handlers/mmio/add_mmio_region.rs

@@ -23,7 +23,7 @@ impl AddMmioRegion {
     }
 
     pub fn handle(self, confidential_flow: ConfidentialFlow) -> ! {
-        match ControlDataStorage::try_confidential_vm(confidential_flow.confidential_vm_id(), |mut confidential_vm| {
+        match ControlDataStorage::try_confidential_vm_mut(confidential_flow.confidential_vm_id(), |mut confidential_vm| {
             ensure!(self.region_start_address % PageSize::Size4KiB.in_bytes() == 0, Error::AddressNotAligned())?;
             ensure!(self.region_length % PageSize::Size4KiB.in_bytes() == 0, Error::AddressNotAligned())?;
             Ok(confidential_vm.add_mmio_region(ConfidentialVmMmioRegion::new(self.region_start_address, self.region_length))?)

security-monitor/src/confidential_flow/handlers/mmio/mmio_access_fault.rs

@@ -26,12 +26,11 @@ impl MmioAccessFault {
     }
 
     pub fn handle(self, mut confidential_flow: ConfidentialFlow) -> ! {
-        match ControlDataStorage::try_confidential_vm_mut(confidential_flow.confidential_vm_id(), |mut confidential_vm| {
+        match ControlDataStorage::try_confidential_vm(confidential_flow.confidential_vm_id(), |confidential_vm| {
             let confidential_vm_physical_address = ConfidentialVmPhysicalAddress::new(self.fault_address);
             let page_size = confidential_vm.memory_protector_mut().map_empty_page(confidential_vm_physical_address, PageSize::Size4KiB)?;
             let request = RemoteHfenceGvmaVmid::all_harts(None, page_size, confidential_flow.confidential_vm_id());
-            confidential_flow
-                .broadcast_remote_command(&mut confidential_vm, ConfidentialHartRemoteCommand::RemoteHfenceGvmaVmid(request))?;
+            confidential_flow.broadcast_remote_command(&confidential_vm, ConfidentialHartRemoteCommand::RemoteHfenceGvmaVmid(request))?;
             Ok(())
         }) {
             Ok(_) => confidential_flow.resume_confidential_hart_execution(),

security-monitor/src/confidential_flow/handlers/mmio/remove_mmio_region.rs

@@ -23,7 +23,7 @@ impl RemoveMmioRegion {
     }
 
     pub fn handle(self, confidential_flow: ConfidentialFlow) -> ! {
-        match ControlDataStorage::try_confidential_vm(confidential_flow.confidential_vm_id(), |mut confidential_vm| {
+        match ControlDataStorage::try_confidential_vm_mut(confidential_flow.confidential_vm_id(), |mut confidential_vm| {
             ensure!(self.region_start_address % PageSize::Size4KiB.in_bytes() == 0, Error::AddressNotAligned())?;
             ensure!(self.region_length % PageSize::Size4KiB.in_bytes() == 0, Error::AddressNotAligned())?;
             Ok(confidential_vm.remove_mmio_region(&ConfidentialVmMmioRegion::new(self.region_start_address, self.region_length)))

security-monitor/src/confidential_flow/handlers/sbi/response.rs

@@ -14,10 +14,6 @@ pub struct SbiResponse {
 }
 
 impl SbiResponse {
-    pub fn from_hypervisor_hart(hypervisor_hart: &HypervisorHart) -> Self {
-        Self { a0: hypervisor_hart.gprs().read(GeneralPurposeRegister::a0), a1: hypervisor_hart.gprs().read(GeneralPurposeRegister::a1) }
-    }
-
     pub fn handle(self, confidential_flow: ConfidentialFlow) -> ! {
         confidential_flow.declassify_and_exit_to_confidential_hart(DeclassifyToConfidentialVm::SbiResponse(self))
     }