acpi: refactor PM1 virtualization

- Refactor read() and write() for ACPIPMResource to protect against PIO
  accesses across PM1 register boundaries, and properly support byte
  accesses.
- If compiled with the feature "direct", all writes to SLP_TYP are
  interpreted as S5 since _Sx is platform-specific.
- Remove Sleep Control and Status Registers since they are only used on
  HW-Reduced ACPI systems.

BUG=b:199383670
TEST=boot Linux kernel and shut down

Change-Id: Iaf9fdf5a161eb6f5618235c3a66f8817258ce289
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3350489
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Tomasz Nowicki <tnowicki@google.com>

devices/src/acpi.rs

@@ -7,18 +7,18 @@ use acpi_tables::{aml, aml::Aml};
 use base::{error, warn, Event};
 
 /// ACPI PM resource for handling OS suspend/resume request
+#[allow(dead_code)]
 pub struct ACPIPMResource {
     suspend_evt: Event,
     exit_evt: Event,
     pm1_status: u16,
     pm1_enable: u16,
     pm1_control: u16,
-    sleep_control: u8,
-    sleep_status: u8,
 }
 
 impl ACPIPMResource {
     /// Constructs ACPI Power Management Resouce.
+    #[allow(dead_code)]
     pub fn new(suspend_evt: Event, exit_evt: Event) -> ACPIPMResource {
         ACPIPMResource {
             suspend_evt,
@@ -26,8 +26,6 @@ impl ACPIPMResource {
             pm1_status: 0,
             pm1_enable: 0,
             pm1_control: 0,
-            sleep_control: 0,
-            sleep_status: 0,
         }
     }
 }
@@ -38,18 +36,36 @@ pub const ACPIPM_RESOURCE_EVENTBLK_LEN: u8 = 4;
 pub const ACPIPM_RESOURCE_CONTROLBLK_LEN: u8 = 2;
 
 /// ACPI PM register value definitions
-const PM1_STATUS: u16 = 0;
-const PM1_ENABLE: u16 = 2;
-const PM1_CONTROL: u16 = 4;
-const SLEEP_CONTROL: u16 = 6;
-const SLEEP_STATUS: u16 = 7;
-const BITMASK_PM1CNT_SLEEP_ENABLE: u16 = 0x2000;
-const BITMASK_SLEEPCNT_SLEEP_ENABLE: u8 = 0x20;
-const BITMASK_PM1CNT_WAKE_STATUS: u16 = 0x8000;
-const BITMASK_SLEEPCNT_WAKE_STATUS: u8 = 0x80;
 
+/// 4.8.4.1.1 PM1 Status Registers, ACPI Spec Version 6.4
+/// Register Location: <PM1a_EVT_BLK / PM1b_EVT_BLK> System I/O or Memory Space (defined in FADT)
+/// Size: PM1_EVT_LEN / 2 (defined in FADT)
+const PM1_STATUS: u16 = 0;
+
+/// 4.8.4.1.2 PM1Enable Registers, ACPI Spec Version 6.4
+/// Register Location: <<PM1a_EVT_BLK / PM1b_EVT_BLK> + PM1_EVT_LEN / 2 System I/O or Memory Space
+/// (defined in FADT)
+/// Size: PM1_EVT_LEN / 2 (defined in FADT)
+const PM1_ENABLE: u16 = PM1_STATUS + (ACPIPM_RESOURCE_EVENTBLK_LEN as u16 / 2);
+
+/// 4.8.4.2.1 PM1 Control Registers, ACPI Spec Version 6.4
+/// Register Location: <PM1a_CNT_BLK / PM1b_CNT_BLK> System I/O or Memory Space (defined in FADT)
+/// Size: PM1_CNT_LEN (defined in FADT)
+const PM1_CONTROL: u16 = PM1_STATUS + ACPIPM_RESOURCE_EVENTBLK_LEN as u16;
+
+const BITMASK_PM1CNT_SLEEP_ENABLE: u16 = 0x2000;
+const BITMASK_PM1CNT_WAKE_STATUS: u16 = 0x8000;
+
+#[cfg(not(feature = "direct"))]
 const BITMASK_PM1CNT_SLEEP_TYPE: u16 = 0x1C00;
-const SLEEP_TYPE_S5: u16 = 0;
+#[cfg(not(feature = "direct"))]
+const SLEEP_TYPE_S1: u16 = 1 << 10;
+#[cfg(not(feature = "direct"))]
+const SLEEP_TYPE_S5: u16 = 0 << 10;
+
+const PM1_STATUS_LAST: u16 = PM1_STATUS + (ACPIPM_RESOURCE_EVENTBLK_LEN as u16 / 2) - 1;
+const PM1_ENABLE_LAST: u16 = PM1_ENABLE + (ACPIPM_RESOURCE_EVENTBLK_LEN as u16 / 2) - 1;
+const PM1_CONTROL_LAST: u16 = PM1_CONTROL + ACPIPM_RESOURCE_CONTROLBLK_LEN as u16 - 1;
 
 impl BusDevice for ACPIPMResource {
     fn debug_label(&self) -> String {
@@ -57,69 +73,116 @@ impl BusDevice for ACPIPMResource {
     }
 
     fn read(&mut self, info: BusAccessInfo, data: &mut [u8]) {
-        let val = match info.offset as u16 {
+        match info.offset as u16 {
-            PM1_STATUS => self.pm1_status,
+            // Accesses to the PM1 registers are done through byte or word accesses
-            PM1_ENABLE => self.pm1_enable,
+            PM1_STATUS..=PM1_STATUS_LAST => {
-            PM1_CONTROL => self.pm1_control,
+                if data.len() > std::mem::size_of::<u16>()
-            SLEEP_CONTROL => self.sleep_control as u16,
+                    || info.offset + data.len() as u64 > (PM1_STATUS_LAST + 1).into()
-            SLEEP_STATUS => self.sleep_status as u16,
+                {
+                    warn!("ACPIPM: bad read size: {}", data.len());
+                    return;
+                }
+                let offset = (info.offset - PM1_STATUS as u64) as usize;
+                data.copy_from_slice(&self.pm1_status.to_ne_bytes()[offset..offset + data.len()]);
+            }
+            PM1_ENABLE..=PM1_ENABLE_LAST => {
+                if data.len() > std::mem::size_of::<u16>()
+                    || info.offset + data.len() as u64 > (PM1_ENABLE_LAST + 1).into()
+                {
+                    warn!("ACPIPM: bad read size: {}", data.len());
+                    return;
+                }
+                let offset = (info.offset - PM1_ENABLE as u64) as usize;
+                data.copy_from_slice(&self.pm1_enable.to_ne_bytes()[offset..offset + data.len()]);
+            }
+            PM1_CONTROL..=PM1_CONTROL_LAST => {
+                if data.len() > std::mem::size_of::<u16>()
+                    || info.offset + data.len() as u64 > (PM1_CONTROL_LAST + 1).into()
+                {
+                    warn!("ACPIPM: bad read size: {}", data.len());
+                    return;
+                }
+                let offset = (info.offset - PM1_CONTROL as u64) as usize;
+                data.copy_from_slice(&self.pm1_control.to_ne_bytes()[offset..offset + data.len()]);
+            }
             _ => {
                 warn!("ACPIPM: Bad read from {}", info);
-                return;
-            }
-        };
-
-        let val_arr = val.to_ne_bytes();
-        for i in 0..std::mem::size_of::<u16>() {
-            if i < data.len() {
-                data[i] = val_arr[i];
             }
         }
     }
 
     fn write(&mut self, info: BusAccessInfo, data: &[u8]) {
-        let max_bytes = std::mem::size_of::<u16>();
-
-        // only allow maximum max_bytes to write
-        if data.len() > max_bytes {
-            warn!("ACPIPM: bad write size: {}", data.len());
-            return;
-        }
-
-        let mut val_arr = u16::to_ne_bytes(0u16);
-        for i in 0..std::mem::size_of::<u16>() {
-            if i < data.len() {
-                val_arr[i] = data[i];
-            }
-        }
-        let val = u16::from_ne_bytes(val_arr);
-
         match info.offset as u16 {
-            PM1_STATUS => self.pm1_status &= !val,
+            // Accesses to the PM1 registers are done through byte or word accesses
-            PM1_ENABLE => self.pm1_enable = val,
+            PM1_STATUS..=PM1_STATUS_LAST => {
-            PM1_CONTROL => {
+                if data.len() > std::mem::size_of::<u16>()
-                if (val & BITMASK_PM1CNT_SLEEP_ENABLE) == BITMASK_PM1CNT_SLEEP_ENABLE {
+                    || info.offset + data.len() as u64 > (PM1_STATUS_LAST + 1).into()
-                    if val & BITMASK_PM1CNT_SLEEP_TYPE == SLEEP_TYPE_S5 {
+                {
-                        if let Err(e) = self.exit_evt.write(1) {
+                    warn!("ACPIPM: bad write size: {}", data.len());
-                            error!("ACPIPM: failed to trigger exit event: {}", e);
+                    return;
+                }
+                let offset = (info.offset - PM1_STATUS as u64) as usize;
+                let mut v = self.pm1_status.to_ne_bytes();
+                for (i, j) in (offset..offset + data.len()).enumerate() {
+                    v[j] &= !data[i];
+                }
+                self.pm1_status = u16::from_ne_bytes(v);
+            }
+            PM1_ENABLE..=PM1_ENABLE_LAST => {
+                if data.len() > std::mem::size_of::<u16>()
+                    || info.offset + data.len() as u64 > (PM1_ENABLE_LAST + 1).into()
+                {
+                    warn!("ACPIPM: bad write size: {}", data.len());
+                    return;
+                }
+                let offset = (info.offset - PM1_ENABLE as u64) as usize;
+                let mut v = self.pm1_enable.to_ne_bytes();
+                for (i, j) in (offset..offset + data.len()).enumerate() {
+                    v[j] = data[i];
+                }
+                self.pm1_enable = u16::from_ne_bytes(v);
+            }
+            PM1_CONTROL..=PM1_CONTROL_LAST => {
+                if data.len() > std::mem::size_of::<u16>()
+                    || info.offset + data.len() as u64 > (PM1_CONTROL_LAST + 1).into()
+                {
+                    warn!("ACPIPM: bad write size: {}", data.len());
+                    return;
+                }
+                let offset = (info.offset - PM1_CONTROL as u64) as usize;
+                let mut v = self.pm1_control.to_ne_bytes();
+                for (i, j) in (offset..offset + data.len()).enumerate() {
+                    v[j] = data[i];
+                }
+                let val = u16::from_ne_bytes(v);
+
+                // SLP_EN is a write-only bit and reads to it always return a zero
+                if (val & BITMASK_PM1CNT_SLEEP_ENABLE) != 0 {
+                    // only support S5 in direct mode
+                    #[cfg(feature = "direct")]
+                    if let Err(e) = self.exit_evt.write(1) {
+                        error!("ACPIPM: failed to trigger exit event: {}", e);
+                    }
+                    #[cfg(not(feature = "direct"))]
+                    match val & BITMASK_PM1CNT_SLEEP_TYPE {
+                        SLEEP_TYPE_S1 => {
+                            if let Err(e) = self.suspend_evt.write(1) {
+                                error!("ACPIPM: failed to trigger suspend event: {}", e);
+                            }
                         }
-                    } else if let Err(e) = self.suspend_evt.write(1) {
+                        SLEEP_TYPE_S5 => {
-                        error!("ACPIPM: failed to trigger suspend event: {}", e);
+                            if let Err(e) = self.exit_evt.write(1) {
+                                error!("ACPIPM: failed to trigger exit event: {}", e);
+                            }
+                        }
+                        _ => error!(
+                            "ACPIPM: unknown SLP_TYP written: {}",
+                            (val & BITMASK_PM1CNT_SLEEP_TYPE) >> 10
+                        ),
                     }
                 }
                 self.pm1_control = val & !BITMASK_PM1CNT_SLEEP_ENABLE;
             }
-            SLEEP_CONTROL => {
-                let sleep_control = val as u8;
-                if (sleep_control & BITMASK_SLEEPCNT_SLEEP_ENABLE) == BITMASK_SLEEPCNT_SLEEP_ENABLE
-                {
-                    if let Err(e) = self.suspend_evt.write(1) {
-                        error!("ACPIPM: failed to trigger suspend event: {}", e);
-                    }
-                }
-                self.sleep_control = sleep_control as u8 & !BITMASK_SLEEPCNT_SLEEP_ENABLE;
-            }
-            SLEEP_STATUS => self.sleep_status &= !val as u8,
             _ => {
                 warn!("ACPIPM: Bad write to {}", info);
             }
@@ -129,11 +192,7 @@ impl BusDevice for ACPIPMResource {
 
 impl BusResumeDevice for ACPIPMResource {
     fn resume_imminent(&mut self) {
-        let val = self.pm1_status;
+        self.pm1_status |= BITMASK_PM1CNT_WAKE_STATUS;
-        self.pm1_status = val | BITMASK_PM1CNT_WAKE_STATUS;
-
-        let val = self.sleep_status;
-        self.sleep_status = val | BITMASK_SLEEPCNT_WAKE_STATUS;
     }
 }