From ebbaa2b63e433e800fc533346b656e3d019ff201 Mon Sep 17 00:00:00 2001
From: Zhigang Wang <wangzhigang17@huawei.com>
Date: Tue, 12 Jan 2021 11:41:52 +0800
Subject: [PATCH 3/3] interrupt_controller: update error-handling of this
 module to use error_chain

1. Update error handling in interrupt_controller module to use error_chain,
   and add corresponding links in device_model's errors.
2. Fix `unwrap()` of return value of function.

Signed-off-by: Zhigang Wang <wangzhigang17@huawei.com>
---
 .../src/interrupt_controller/aarch64/gicv3.rs | 39 ++++-------
 .../src/interrupt_controller/aarch64/mod.rs   | 67 +++++++++++++------
 device_model/src/interrupt_controller/mod.rs  |  2 +-
 device_model/src/lib.rs                       |  2 +
 device_model/src/micro_vm/mod.rs              |  1 +
 5 files changed, 64 insertions(+), 47 deletions(-)

diff --git a/device_model/src/interrupt_controller/aarch64/gicv3.rs b/device_model/src/interrupt_controller/aarch64/gicv3.rs
index 4753b2e..2d11740 100644
--- a/device_model/src/interrupt_controller/aarch64/gicv3.rs
+++ b/device_model/src/interrupt_controller/aarch64/gicv3.rs
@@ -15,10 +15,10 @@ use std::sync::{Arc, Mutex};
 use kvm_ioctls::{DeviceFd, VmFd};
 
 use machine_manager::machine::{KvmVmState, MachineLifecycle};
-use util::{device_tree, errors};
+use util::device_tree;
 
-use super::GICConfig;
-use super::GICDevice;
+use super::errors::{ErrorKind, Result};
+use super::{GICConfig, GICDevice, UtilResult};
 
 use crate::{LayoutEntryType, MEM_LAYOUT};
 
@@ -26,21 +26,6 @@ use crate::{LayoutEntryType, MEM_LAYOUT};
 const SZ_64K: u64 = 0x0001_0000;
 const KVM_VGIC_V3_REDIST_SIZE: u64 = 2 * SZ_64K;
 
-#[derive(Debug)]
-pub enum Error {
-    /// Error while calling KVM ioctl for setting up the global interrupt controller.
-    CreateGIC(kvm_ioctls::Error),
-    /// Error while setting device attributes for the GIC.
-    SetDeviceAttribute(kvm_ioctls::Error),
-    /// Error while getting device attributes for the GIC.
-    GetDeviceAttribute(kvm_ioctls::Error),
-    /// Error while check device attributes for the GIC.
-    CheckDeviceAttribute(kvm_ioctls::Error),
-    /// Error while multiple redistributor is acquired but KVM does not support it.
-    MultiRedistributor,
-}
-type Result<T> = std::result::Result<T, Error>;
-
 /// A wrapper for kvm_based device check and access.
 pub struct KvmDevice;
 
@@ -55,7 +40,7 @@ impl KvmDevice {
 
         Ok(fd
             .has_device_attr(&attr)
-            .map_err(Error::CheckDeviceAttribute)?)
+            .map_err(ErrorKind::CheckDeviceAttribute)?)
     }
 
     fn kvm_device_access(
@@ -74,11 +59,11 @@ impl KvmDevice {
 
         if write {
             fd.set_device_attr(&attr)
-                .map_err(Error::SetDeviceAttribute)?;
+                .map_err(ErrorKind::SetDeviceAttribute)?;
         } else {
             let mut attr = attr;
             fd.get_device_attr(&mut attr)
-                .map_err(Error::GetDeviceAttribute)?;
+                .map_err(ErrorKind::GetDeviceAttribute)?;
         };
 
         Ok(())
@@ -144,7 +129,7 @@ pub struct GICv3 {
 
 impl GICv3 {
     pub fn new(vm: &Arc<VmFd>, config: &GICConfig) -> Result<Self> {
-        config.check_sanity().unwrap();
+        config.check_sanity()?;
 
         let mut gic_device = kvm_bindings::kvm_create_device {
             type_: kvm_bindings::kvm_device_type_KVM_DEV_TYPE_ARM_VGIC_V3,
@@ -154,7 +139,7 @@ impl GICv3 {
 
         let gic_fd = match vm.create_device(&mut gic_device) {
             Ok(fd) => fd,
-            Err(e) => return Err(Error::CreateGIC(e)),
+            Err(e) => return Err(ErrorKind::CreateGIC(e).into()),
         };
 
         // Calculate GIC redistributor regions' address range according to vcpu count.
@@ -193,7 +178,7 @@ impl GICv3 {
         };
 
         if gicv3.its {
-            gicv3.its_dev = Some(GICv3Its::new(&vm).unwrap());
+            gicv3.its_dev = Some(GICv3Its::new(&vm)?);
         }
 
         Ok(gicv3)
@@ -206,7 +191,7 @@ impl GICv3 {
                 kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ADDR,
                 kvm_bindings::KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION as u64,
             )
-            .map_err(|_| Error::MultiRedistributor)?;
+            .map_err(|_| ErrorKind::MultiRedistributor)?;
         }
 
         if self.redist_regions.len() == 1 {
@@ -393,7 +378,7 @@ impl GICDevice for GICv3 {
         Ok(())
     }
 
-    fn generate_fdt(&self, fdt: &mut Vec<u8>) -> errors::Result<()> {
+    fn generate_fdt(&self, fdt: &mut Vec<u8>) -> UtilResult<()> {
         let redist_count = self.redist_regions.len() as u32;
         let mut gic_reg = vec![self.dist_base, self.dist_size];
 
@@ -457,7 +442,7 @@ impl GICv3Its {
 
         let its_fd = match vm.create_device(&mut its_device) {
             Ok(fd) => fd,
-            Err(e) => return Err(Error::CreateGIC(e)),
+            Err(e) => return Err(ErrorKind::CreateGIC(e).into()),
         };
 
         Ok(GICv3Its {
diff --git a/device_model/src/interrupt_controller/aarch64/mod.rs b/device_model/src/interrupt_controller/aarch64/mod.rs
index 03c336b..14b9a8f 100644
--- a/device_model/src/interrupt_controller/aarch64/mod.rs
+++ b/device_model/src/interrupt_controller/aarch64/mod.rs
@@ -16,20 +16,43 @@ use std::sync::Arc;
 
 use kvm_ioctls::VmFd;
 
-pub use gicv3::Error as GICError;
 pub use gicv3::GICv3;
 use machine_manager::machine::{KvmVmState, MachineLifecycle};
-use util::{device_tree, errors};
+use util::{device_tree, errors::Result as UtilResult};
 
 // First 32 are private to each CPU (SGIs and PPIs).
 const GIC_IRQ_INTERNAL: u32 = 32;
 
-#[derive(Debug)]
-pub enum Error {
-    /// Invalid argument
-    EINVAL(std::string::String),
+pub mod errors {
+    error_chain! {
+        foreign_links {
+            Kvm(kvm_ioctls::Error);
+        }
+        errors {
+            InvalidConfig(err_info: String) {
+                display("Invalid config: {}.", err_info)
+            }
+            CreateGIC(err: kvm_ioctls::Error) {
+                display("Failed to create GIC device: {:#?}.", err)
+            }
+            SetDeviceAttribute(err: kvm_ioctls::Error) {
+                display("Failed to set device attributes for GIC: {:#?}.", err)
+            }
+            GetDeviceAttribute(err: kvm_ioctls::Error) {
+                display("Failed to get device attributes for GIC: {:#?}.", err)
+            }
+            CheckDeviceAttribute(err: kvm_ioctls::Error) {
+                display("Failed to check device attributes for GIC: {:#?}.", err)
+            }
+            MultiRedistributor {
+                display("multiple redistributor is acquired but KVM does not support it.")
+            }
+        }
+    }
 }
 
+use self::errors::{ErrorKind, Result};
+
 /// Configure a Interrupt controller.
 pub struct GICConfig {
     /// Config GIC version
@@ -43,19 +66,21 @@ pub struct GICConfig {
 }
 
 impl GICConfig {
-    fn check_sanity(&self) -> Result<(), Error> {
+    fn check_sanity(&self) -> Result<()> {
         if self.version != kvm_bindings::kvm_device_type_KVM_DEV_TYPE_ARM_VGIC_V3 {
-            return Err(Error::EINVAL("GIC only support GICv3".to_string()));
+            return Err(ErrorKind::InvalidConfig("GIC only support GICv3".to_string()).into());
         };
 
         if self.vcpu_count > 256 || self.vcpu_count == 0 {
-            return Err(Error::EINVAL(
-                "GIC only support maximum 256 vcpus".to_string(),
-            ));
+            return Err(
+                ErrorKind::InvalidConfig("GIC only support maximum 256 vcpus".to_string()).into(),
+            );
         }
 
         if self.max_irq <= GIC_IRQ_INTERNAL {
-            return Err(Error::EINVAL("GIC irq numbers need above 32".to_string()));
+            return Err(
+                ErrorKind::InvalidConfig("GIC irq numbers need above 32".to_string()).into(),
+            );
         }
 
         Ok(())
@@ -73,19 +98,19 @@ pub trait GICDevice: MachineLifecycle {
     fn create_device(
         vm: &Arc<VmFd>,
         gic_conf: &GICConfig,
-    ) -> Result<Arc<dyn GICDevice + std::marker::Send + std::marker::Sync>, GICError>
+    ) -> Result<Arc<dyn GICDevice + std::marker::Send + std::marker::Sync>>
     where
         Self: Sized;
 
     /// Realize function for kvm_based `GIC` device.
-    fn realize(&self) -> Result<(), GICError>;
+    fn realize(&self) -> Result<()>;
 
     /// Constructs `fdt` node for `GIC`.
     ///
     /// # Arguments
     ///
     /// * `fdt` - Device tree presented by bytes.
-    fn generate_fdt(&self, fdt: &mut Vec<u8>) -> errors::Result<()>;
+    fn generate_fdt(&self, fdt: &mut Vec<u8>) -> UtilResult<()>;
 }
 
 /// A wrapper around creating and using a kvm-based interrupt controller.
@@ -100,12 +125,17 @@ impl InterruptController {
     ///
     /// * `vm` - File descriptor for vmfd.
     /// * `gic_conf` - Configuration for `GIC`.
-    pub fn new(vm: Arc<VmFd>, gic_conf: &GICConfig) -> Result<InterruptController, std::io::Error> {
+    pub fn new(vm: Arc<VmFd>, gic_conf: &GICConfig) -> Result<InterruptController> {
         Ok(InterruptController {
-            gic: GICv3::create_device(&vm, gic_conf).unwrap(),
+            gic: GICv3::create_device(&vm, gic_conf)?,
         })
     }
 
+    pub fn realize(&self) -> Result<()> {
+        self.gic.realize()?;
+        Ok(())
+    }
+
     /// Change `InterruptController` lifecycle state to `Stopped`.
     pub fn stop(&self) {
         self.gic
@@ -114,9 +144,8 @@ impl InterruptController {
     }
 }
 
-#[cfg(target_arch = "aarch64")]
 impl device_tree::CompileFDT for InterruptController {
-    fn generate_fdt_node(&self, fdt: &mut Vec<u8>) -> errors::Result<()> {
+    fn generate_fdt_node(&self, fdt: &mut Vec<u8>) -> UtilResult<()> {
         self.gic.generate_fdt(fdt)?;
         debug!("Interrupt Controller device tree generated!");
         Ok(())
diff --git a/device_model/src/interrupt_controller/mod.rs b/device_model/src/interrupt_controller/mod.rs
index c94cc19..83a5ba3 100644
--- a/device_model/src/interrupt_controller/mod.rs
+++ b/device_model/src/interrupt_controller/mod.rs
@@ -27,7 +27,7 @@
 mod aarch64;
 
 #[cfg(target_arch = "aarch64")]
-pub use aarch64::InterruptController;
+pub use aarch64::{errors, InterruptController};
 
 #[cfg(target_arch = "aarch64")]
 pub use aarch64::GICConfig as InterruptControllerConfig;
diff --git a/device_model/src/lib.rs b/device_model/src/lib.rs
index 06d742e..4701bdb 100644
--- a/device_model/src/lib.rs
+++ b/device_model/src/lib.rs
@@ -79,6 +79,8 @@ pub mod errors {
             Manager(machine_manager::errors::Error, machine_manager::errors::ErrorKind);
             Cpu(crate::cpu::errors::Error, crate::cpu::errors::ErrorKind);
             Mmio(crate::mmio::errors::Error, crate::mmio::errors::ErrorKind);
+            InterruptController(crate::interrupt_controller::errors::Error,
+                crate::interrupt_controller::errors::ErrorKind) #[cfg(target_arch = "aarch64")];
         }
         foreign_links {
             Io(std::io::Error);
diff --git a/device_model/src/micro_vm/mod.rs b/device_model/src/micro_vm/mod.rs
index 63db895..94a0404 100644
--- a/device_model/src/micro_vm/mod.rs
+++ b/device_model/src/micro_vm/mod.rs
@@ -353,6 +353,7 @@ impl LightMachine {
     /// Realize `LightMachine` means let all members of `LightMachine` enabled.
     #[cfg(target_arch = "aarch64")]
     pub fn realize(&self) -> Result<()> {
+        self.irq_chip.realize()?;
         self.bus
             .realize_devices(&self.vm_fd, &self.boot_source, &self.sys_mem)?;
 
-- 
2.27.0

