devices, vmm: Use a bit field for ACPI GED interrupt type

Use independent bits for storing whether there is a CPU or memory device
changed when reporting changes via ACPI GED interrupt. This prevents a
later notification squashing an earlier one and ensure that hotplugging
both CPU and memory at the same time succeeds.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>

Cargo.lock

@@ -234,6 +234,7 @@ dependencies = [
 name = "devices"
 version = "0.1.0"
 dependencies = [
+ "bitflags 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "epoll 4.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "kvm-bindings 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",

devices/Cargo.toml

@@ -4,6 +4,7 @@ version = "0.1.0"
 authors = ["The Chromium OS Authors"]
 
 [dependencies]
+bitflags = ">=1.2.1"
 byteorder = "1.3.2"
 epoll = ">=4.0.1"
 kvm-bindings = "0.2.0"

devices/src/acpi.rs

@@ -5,7 +5,7 @@
 
 use vmm_sys_util::eventfd::EventFd;
 use BusDevice;
-use HotPlugNotificationType;
+use HotPlugNotificationFlags;
 use Interrupt;
 
 /// A device for handling ACPI shutdown and reboot
@@ -46,6 +46,7 @@ impl BusDevice for AcpiShutdownDevice {
         const SLEEP_VALUE_BIT: u8 = 2;
         if data[0] == (S5_SLEEP_VALUE << SLEEP_VALUE_BIT) | (1 << SLEEP_STATUS_EN_BIT) {
             debug!("ACPI Shutdown signalled");
+            extern crate bitflags;
             if let Err(e) = self.exit_evt.write(1) {
                 error!("Error triggering ACPI shutdown event: {}", e);
             }
@@ -56,7 +57,7 @@ impl BusDevice for AcpiShutdownDevice {
 /// A device for handling ACPI GED event generation
 pub struct AcpiGEDDevice {
     interrupt: Box<dyn Interrupt>,
-    notification_type: HotPlugNotificationType,
+    notification_type: HotPlugNotificationFlags,
     ged_irq: u32,
 }
 
@@ -64,16 +65,16 @@ impl AcpiGEDDevice {
     pub fn new(interrupt: Box<dyn Interrupt>, ged_irq: u32) -> AcpiGEDDevice {
         AcpiGEDDevice {
             interrupt,
-            notification_type: HotPlugNotificationType::NoDevicesChanged,
+            notification_type: HotPlugNotificationFlags::NO_DEVICES_CHANGED,
             ged_irq,
         }
     }
 
     pub fn notify(
         &mut self,
-        notification_type: HotPlugNotificationType,
+        notification_type: HotPlugNotificationFlags,
     ) -> Result<(), std::io::Error> {
-        self.notification_type = notification_type;
+        self.notification_type |= notification_type;
         self.interrupt.deliver()
     }
 
@@ -86,8 +87,8 @@ impl AcpiGEDDevice {
 impl BusDevice for AcpiGEDDevice {
     // Spec has all fields as zero
     fn read(&mut self, _base: u64, _offset: u64, data: &mut [u8]) {
-        data[0] = self.notification_type as u8;
-        self.notification_type = HotPlugNotificationType::NoDevicesChanged;
+        data[0] = self.notification_type.bits();
+        self.notification_type = HotPlugNotificationFlags::NO_DEVICES_CHANGED;
     }
 
     fn write(&mut self, _base: u64, _offset: u64, _data: &[u8]) {}

devices/src/lib.rs

@@ -6,6 +6,8 @@
 // found in the LICENSE-BSD-3-Clause file.
 
 //! Emulates virtual and hardware devices.
+#[macro_use]
+extern crate bitflags;
 extern crate byteorder;
 extern crate epoll;
 extern crate kvm_bindings;
@@ -71,9 +73,10 @@ pub trait Interrupt: Send + Sync {
     fn deliver(&self) -> result::Result<(), std::io::Error>;
 }
 
-#[derive(Clone, Copy)]
-pub enum HotPlugNotificationType {
-    NoDevicesChanged,
-    CPUDevicesChanged,
-    MemoryDevicesChanged,
+bitflags! {
+    pub struct HotPlugNotificationFlags: u8 {
+        const NO_DEVICES_CHANGED = 0;
+        const CPU_DEVICES_CHANGED = 0b1;
+        const MEMORY_DEVICES_CHANGED = 0b10;
+    }
 }

vmm/src/device_manager.rs

@@ -19,7 +19,7 @@ use acpi_tables::{aml, aml::Aml};
 use arc_swap::ArcSwap;
 use arch::layout;
 use arch::layout::{APIC_START, IOAPIC_SIZE, IOAPIC_START};
-use devices::{ioapic, HotPlugNotificationType};
+use devices::{ioapic, HotPlugNotificationFlags};
 use kvm_bindings::kvm_irq_routing_entry;
 use kvm_ioctls::*;
 use libc::O_TMPFILE;
@@ -1618,7 +1618,7 @@ impl DeviceManager {
 
     pub fn notify_hotplug(
         &self,
-        _notification_type: HotPlugNotificationType,
+        _notification_type: HotPlugNotificationFlags,
     ) -> DeviceManagerResult<()> {
         #[cfg(feature = "acpi")]
         return self
@@ -1670,12 +1670,14 @@ fn create_ged_device(ged_irq: u32) -> Vec<u8> {
                 true,
                 vec![
                     &aml::Store::new(&aml::Local(0), &aml::Path::new("GDAT")),
+                    &aml::And::new(&aml::Local(1), &aml::Local(0), &aml::ONE),
                     &aml::If::new(
-                        &aml::Equal::new(&aml::Local(0), &aml::ONE),
+                        &aml::Equal::new(&aml::Local(1), &aml::ONE),
                         vec![&aml::MethodCall::new("\\_SB_.CPUS.CSCN".into(), vec![])],
                     ),
+                    &aml::And::new(&aml::Local(1), &aml::Local(0), &2usize),
                     &aml::If::new(
-                        &aml::Equal::new(&aml::Local(0), &2usize),
+                        &aml::Equal::new(&aml::Local(1), &2usize),
                         vec![&aml::MethodCall::new("\\_SB_.MHPC.MSCN".into(), vec![])],
                     ),
                 ],

vmm/src/vm.rs

@@ -31,7 +31,7 @@ use crate::memory_manager::{get_host_cpu_phys_bits, Error as MemoryManagerError,
 use anyhow::anyhow;
 use arc_swap::ArcSwap;
 use arch::layout;
-use devices::{ioapic, HotPlugNotificationType};
+use devices::{ioapic, HotPlugNotificationFlags};
 use kvm_bindings::{kvm_enable_cap, kvm_userspace_memory_region, KVM_CAP_SPLIT_IRQCHIP};
 use kvm_ioctls::*;
 use linux_loader::cmdline::Cmdline;
@@ -491,7 +491,7 @@ impl Vm {
                 .resize(desired_vcpus)
                 .map_err(Error::CpuManager)?;
             self.devices
-                .notify_hotplug(HotPlugNotificationType::CPUDevicesChanged)
+                .notify_hotplug(HotPlugNotificationFlags::CPU_DEVICES_CHANGED)
                 .map_err(Error::DeviceManager)?;
             self.config.lock().unwrap().cpus.boot_vcpus = desired_vcpus;
         }
@@ -503,7 +503,7 @@ impl Vm {
                 .resize(desired_memory)
                 .map_err(Error::MemoryManager)?;
             self.devices
-                .notify_hotplug(HotPlugNotificationType::MemoryDevicesChanged)
+                .notify_hotplug(HotPlugNotificationFlags::MEMORY_DEVICES_CHANGED)
                 .map_err(Error::DeviceManager)?;
             self.config.lock().unwrap().memory.size = desired_memory;
         }