[Xen-devel] [PATCH 4/6] ACPI: support v5 (reduced HW) sleep interface

["Jan Beulich" <JBeulich@xxxxxxxx>]

Note that this also fixes a broken input check in acpi_enter_sleep()
(previously validating the sleep->pm1[ab]_cnt_val relationship based
on acpi_sinfo.pm1b_cnt_val, which however gets set only subsequently).

Also adjust a few minor issues with the pre-v5 handling in
acpi_fadt_parse_sleep_info().

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
A question is whether, rather than passing the raw values read from
_Sx to XENPF_enter_acpi_sleep, we should again have Dom0 pass the
values ready to be written into the registers - that would make things
consistent with the legacy case, but add further dependency of the
hypervisor on Dom0 behavior where it's not really needed. I think that
it was a mistake for the original variant to have Dom0 pass massaged
values to the hypervisor.

--- a/xen/arch/x86/acpi/boot.c
+++ b/xen/arch/x86/acpi/boot.c
@@ -310,14 +310,15 @@ static int __init acpi_invalidate_bgrt(s
 
 #ifdef CONFIG_ACPI_SLEEP
 #define acpi_fadt_copy_address(dst, src, len) do {                     \
-       if (fadt->header.revision >= FADT2_REVISION_ID)                 \
+       if (fadt->header.revision >= FADT2_REVISION_ID &&               \
+           fadt->header.length >= ACPI_FADT_V2_SIZE)                   \
                acpi_sinfo.dst##_blk = fadt->x##src##_block;            \
        if (!acpi_sinfo.dst##_blk.address) {                            \
                acpi_sinfo.dst##_blk.address      = fadt->src##_block;  \
                acpi_sinfo.dst##_blk.space_id     = ACPI_ADR_SPACE_SYSTEM_IO; \
                acpi_sinfo.dst##_blk.bit_width    = fadt->len##_length << 3; \
                acpi_sinfo.dst##_blk.bit_offset   = 0;                  \
-               acpi_sinfo.dst##_blk.access_width = 0;                  \
+               acpi_sinfo.dst##_blk.access_width = fadt->len##_length; \
        } \
 } while (0)
 
@@ -328,6 +329,41 @@ acpi_fadt_parse_sleep_info(struct acpi_t
        struct acpi_table_facs *facs = NULL;
        uint64_t facs_pa;
 
+       if (fadt->header.revision >= 5 &&
+           fadt->header.length >= ACPI_FADT_V5_SIZE) {
+               acpi_sinfo.sleep_control = fadt->sleep_control;
+               acpi_sinfo.sleep_status = fadt->sleep_status;
+
+               printk(KERN_INFO PREFIX
+                      "v5 SLEEP INFO: control[%d:%"PRIx64"],"
+                      " status[%d:%"PRIx64"]\n",
+                      acpi_sinfo.sleep_control.space_id,
+                      acpi_sinfo.sleep_control.address,
+                      acpi_sinfo.sleep_status.space_id,
+                      acpi_sinfo.sleep_status.address);
+
+               if ((fadt->sleep_control.address &&
+                    (fadt->sleep_control.bit_offset ||
+                     fadt->sleep_control.bit_width !=
+                     fadt->sleep_control.access_width * 8)) ||
+                   (fadt->sleep_status.address &&
+                    (fadt->sleep_status.bit_offset ||
+                     fadt->sleep_status.bit_width !=
+                     fadt->sleep_status.access_width * 8))) {
+                       printk(KERN_WARNING PREFIX
+                              "Invalid sleep control/status register data:"
+                              " %#x:%#x:%#x %#x:%#x:%#x\n",
+                              fadt->sleep_control.bit_offset,
+                              fadt->sleep_control.bit_width,
+                              fadt->sleep_control.access_width,
+                              fadt->sleep_status.bit_offset,
+                              fadt->sleep_status.bit_width,
+                              fadt->sleep_status.access_width);
+                       fadt->sleep_control.address = 0;
+                       fadt->sleep_status.address = 0;
+               }
+       }
+
        if (fadt->flags & ACPI_FADT_HW_REDUCED)
                goto bad;
 
@@ -337,7 +373,7 @@ acpi_fadt_parse_sleep_info(struct acpi_t
        acpi_fadt_copy_address(pm1b_evt, pm1b_event, pm1_event);
 
        printk(KERN_INFO PREFIX
-              "ACPI SLEEP INFO: pm1x_cnt[%"PRIx64",%"PRIx64"], "
+              "SLEEP INFO: pm1x_cnt[%"PRIx64",%"PRIx64"], "
               "pm1x_evt[%"PRIx64",%"PRIx64"]\n",
               acpi_sinfo.pm1a_cnt_blk.address,
               acpi_sinfo.pm1b_cnt_blk.address,
@@ -384,11 +420,14 @@ acpi_fadt_parse_sleep_info(struct acpi_t
        acpi_sinfo.vector_width = 32;
 
        printk(KERN_INFO PREFIX
-              "                 wakeup_vec[%"PRIx64"], vec_size[%x]\n",
+              "            wakeup_vec[%"PRIx64"], vec_size[%x]\n",
               acpi_sinfo.wakeup_vector, acpi_sinfo.vector_width);
        return;
 bad:
-       memset(&acpi_sinfo, 0, sizeof(acpi_sinfo));
+       memset(&acpi_sinfo, 0,
+              offsetof(struct acpi_sleep_info, sleep_control));
+       memset(&acpi_sinfo.sleep_status + 1, 0,
+              (long)(&acpi_sinfo + 1) - (long)(&acpi_sinfo.sleep_status + 1));
 }
 #endif
 
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -239,23 +239,47 @@ static long enter_state_helper(void *dat
  */
 int acpi_enter_sleep(struct xenpf_enter_acpi_sleep *sleep)
 {
-    if ( !acpi_sinfo.pm1a_cnt_blk.address )
+    if ( sleep->flags & XENPF_ACPI_SLEEP_EXTENDED )
+    {
+        if ( !acpi_sinfo.sleep_control.address ||
+             !acpi_sinfo.sleep_status.address )
+            return -EPERM;
+
+        if ( sleep->flags & ~XENPF_ACPI_SLEEP_EXTENDED )
+            return -EINVAL;
+
+        if ( sleep->val_a > ACPI_SLEEP_TYPE_MAX ||
+             (sleep->val_b != ACPI_SLEEP_TYPE_INVALID &&
+              sleep->val_b > ACPI_SLEEP_TYPE_MAX) )
+            return -ERANGE;
+
+        acpi_sinfo.sleep_type_a = sleep->val_a;
+        acpi_sinfo.sleep_type_b = sleep->val_b;
+
+        acpi_sinfo.sleep_extended = 1;
+    }
+
+    else if ( !acpi_sinfo.pm1a_cnt_blk.address )
         return -EPERM;
 
     /* Sanity check */
-    if ( acpi_sinfo.pm1b_cnt_val &&
-         ((sleep->pm1a_cnt_val ^ sleep->pm1b_cnt_val) &
-          ACPI_BITMASK_SLEEP_ENABLE) )
+    else if ( sleep->val_b &&
+              ((sleep->val_a ^ sleep->val_b) & ACPI_BITMASK_SLEEP_ENABLE) )
     {
         gdprintk(XENLOG_ERR, "Mismatched pm1a/pm1b setting.");
         return -EINVAL;
     }
 
-    if ( sleep->flags )
+    else if ( sleep->flags )
         return -EINVAL;
 
-    acpi_sinfo.pm1a_cnt_val = sleep->pm1a_cnt_val;
-    acpi_sinfo.pm1b_cnt_val = sleep->pm1b_cnt_val;
+    else
+    {
+        acpi_sinfo.pm1a_cnt_val = sleep->val_a;
+        acpi_sinfo.pm1b_cnt_val = sleep->val_b;
+        acpi_sinfo.sleep_extended = 0;
+    }
+
     acpi_sinfo.sleep_state = sleep->sleep_state;
 
     return continue_hypercall_on_cpu(0, enter_state_helper, &acpi_sinfo);
@@ -266,6 +290,13 @@ static int acpi_get_wake_status(void)
     uint32_t val;
     acpi_status status;
 
+    if ( acpi_sinfo.sleep_extended )
+    {
+        status = acpi_hw_register_read(ACPI_REGISTER_SLEEP_STATUS, &val);
+
+        return ACPI_FAILURE(status) ? 0 : val & ACPI_X_WAKE_STATUS;
+    }
+
     /* Wake status is the 15th bit of PM1 status register. (ACPI spec 3.0) */
     status = acpi_hw_register_read(ACPI_REGISTER_PM1_STATUS, &val);
     if ( ACPI_FAILURE(status) )
@@ -335,19 +366,33 @@ acpi_status acpi_enter_sleep_state(u8 sl
 
     ACPI_FLUSH_CPU_CACHE();
 
-    status = acpi_hw_register_write(ACPI_REGISTER_PM1A_CONTROL, 
-                                    acpi_sinfo.pm1a_cnt_val);
-    if ( ACPI_FAILURE(status) )
-        return_ACPI_STATUS(AE_ERROR);
-
-    if ( acpi_sinfo.pm1b_cnt_blk.address )
+    if ( acpi_sinfo.sleep_extended )
     {
-        status = acpi_hw_register_write(ACPI_REGISTER_PM1B_CONTROL, 
-                                        acpi_sinfo.pm1b_cnt_val);
-        if ( ACPI_FAILURE(status) )
-            return_ACPI_STATUS(AE_ERROR);
+        /*
+         * Set the SLP_TYP and SLP_EN bits.
+         *
+         * Note: We only use the first value returned by the \_Sx method
+         * (acpi_sinfo.sleep_type_a) - As per ACPI specification.
+         */
+        u8 sleep_type_value =
+            ((acpi_sinfo.sleep_type_a << ACPI_X_SLEEP_TYPE_POSITION) &
+             ACPI_X_SLEEP_TYPE_MASK) | ACPI_X_SLEEP_ENABLE;
+
+        status = acpi_hw_register_write(ACPI_REGISTER_SLEEP_CONTROL,
+                                        sleep_type_value);
+    }
+    else
+    {
+        status = acpi_hw_register_write(ACPI_REGISTER_PM1A_CONTROL,
+                                        acpi_sinfo.pm1a_cnt_val);
+        if ( !ACPI_FAILURE(status) && acpi_sinfo.pm1b_cnt_blk.address )
+            status = acpi_hw_register_write(ACPI_REGISTER_PM1B_CONTROL,
+                                            acpi_sinfo.pm1b_cnt_val);
     }
 
+    if ( ACPI_FAILURE(status) )
+        return_ACPI_STATUS(AE_ERROR);
+
     /* Wait until we enter sleep state, and spin until we wake */
     while ( !acpi_get_wake_status() )
         continue;
--- a/xen/drivers/acpi/hwregs.c
+++ b/xen/drivers/acpi/hwregs.c
@@ -365,6 +365,14 @@ acpi_hw_register_read(u32 register_id, u
                    acpi_os_read_port(acpi_gbl_FADT.smi_command, &value1, 8);
                break;
 
+       case ACPI_REGISTER_SLEEP_STATUS:
+
+               status =
+                   acpi_hw_low_level_read(acpi_gbl_FADT.sleep_status.bit_width,
+                                          &value1,
+                                          &acpi_gbl_FADT.sleep_status);
+               break;
+
        default:
                ACPI_DEBUG_PRINT((AE_INFO, "Unknown Register ID: %X", 
register_id));
                status = AE_BAD_PARAMETER;
@@ -525,6 +533,14 @@ acpi_status acpi_hw_register_write(u32 r
                    acpi_os_write_port(acpi_gbl_FADT.smi_command, value, 8);
                break;
 
+       case ACPI_REGISTER_SLEEP_CONTROL:
+
+               status =
+                   
acpi_hw_low_level_write(acpi_gbl_FADT.sleep_control.bit_width,
+                                           value,
+                                           &acpi_gbl_FADT.sleep_control);
+               break;
+
        default:
                status = AE_BAD_PARAMETER;
                break;
--- a/xen/include/acpi/aclocal.h
+++ b/xen/include/acpi/aclocal.h
@@ -128,6 +128,8 @@ struct acpi_bit_register_info {
 #define ACPI_REGISTER_PM_TIMER                  0x07
 #define ACPI_REGISTER_PROCESSOR_BLOCK           0x08
 #define ACPI_REGISTER_SMI_COMMAND_BLOCK         0x09
+#define ACPI_REGISTER_SLEEP_CONTROL             0x0a
+#define ACPI_REGISTER_SLEEP_STATUS              0x0b
 
 /* Masks used to access the bit_registers */
 
--- a/xen/include/acpi/actbl.h
+++ b/xen/include/acpi/actbl.h
@@ -309,6 +309,13 @@ enum acpi_prefered_pm_profiles {
        PM_TABLET = 8
 };
 
+/* Values for sleep_status and sleep_control registers (V5 FADT) */
+
+#define ACPI_X_WAKE_STATUS          0x80
+#define ACPI_X_SLEEP_TYPE_MASK      0x1C
+#define ACPI_X_SLEEP_TYPE_POSITION  0x02
+#define ACPI_X_SLEEP_ENABLE         0x20
+
 /* Reset to default packing */
 
 #pragma pack()
--- a/xen/include/asm-x86/acpi.h
+++ b/xen/include/asm-x86/acpi.h
@@ -126,11 +126,20 @@ struct acpi_sleep_info {
     struct acpi_generic_address pm1b_cnt_blk;
     struct acpi_generic_address pm1a_evt_blk;
     struct acpi_generic_address pm1b_evt_blk;
-    uint16_t pm1a_cnt_val;
-    uint16_t pm1b_cnt_val;
+    struct acpi_generic_address sleep_control;
+    struct acpi_generic_address sleep_status;
+    union {
+        uint16_t pm1a_cnt_val;
+        uint8_t sleep_type_a;
+    };
+    union {
+        uint16_t pm1b_cnt_val;
+        uint8_t sleep_type_b;
+    };
     uint32_t sleep_state;
     uint64_t wakeup_vector;
     uint32_t vector_width;
+    bool_t sleep_extended;
 };
 
 #endif /* CONFIG_ACPI_SLEEP */
--- a/xen/include/public/platform.h
+++ b/xen/include/public/platform.h
@@ -290,10 +290,16 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_firmware_i
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
     /* IN variables */
+#if __XEN_INTERFACE_VERSION__ < 0x00040300
     uint16_t pm1a_cnt_val;      /* PM1a control value. */
     uint16_t pm1b_cnt_val;      /* PM1b control value. */
+#else
+    uint16_t val_a;             /* PM1a control / sleep type A. */
+    uint16_t val_b;             /* PM1b control / sleep type B. */
+#endif
     uint32_t sleep_state;       /* Which state to enter (Sn). */
-    uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+    uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 typedef struct xenpf_enter_acpi_sleep xenpf_enter_acpi_sleep_t;
 DEFINE_XEN_GUEST_HANDLE(xenpf_enter_acpi_sleep_t);

Attachment: ACPI-v5-sleep.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel