Re: [Xen-devel] [PATCH] tools/firmware: add ACPI device for Windows laptop/slate mode switch

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@xxxxxxxxxx]
> Sent: 24 March 2017 10:03
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Owen Smith
> <owen.smith@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>
> Subject: [PATCH] tools/firmware: add ACPI device for Windows laptop/slate
> mode switch
> 
> Microsoft have defined an ACPI device to support switching Windows 10
> between laptop/desktop mode and slate/tablet mode [1].
> 
> This patch adds an SSDT containing such a device. The presence of the
> device is controlled by a new 'acpi_conv' boolean in xl.cfg. The new
> device will not be present by default.
> 
> [1] https://msdn.microsoft.com/en-
> us/windows/hardware/commercialize/design/device-
> experiences/continuum
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> NOTE: Odd looking braces style in tools/libacpi/build.c is for
>       consistency with existing code.
> ---
>  docs/man/xl.cfg.pod.5.in        |  5 +++++
>  tools/firmware/hvmloader/util.c |  2 ++
>  tools/libacpi/Makefile          |  4 ++--
>  tools/libacpi/build.c           | 11 +++++++++++
>  tools/libacpi/libacpi.h         |  1 +
>  tools/libacpi/ssdt_conv.asl     | 38
> ++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_create.c      |  3 +++
>  tools/libxl/libxl_types.idl     |  1 +
>  tools/xl/xl_parse.c             |  1 +
>  9 files changed, 64 insertions(+), 2 deletions(-)
>  create mode 100644 tools/libacpi/ssdt_conv.asl
> 
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index 52802d5..6f57aa9 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -1256,6 +1256,11 @@ ACPI table. True (1) by default.
>  Include S4 (suspend-to-disk) power state in the virtual firmware ACPI
>  table. True (1) by default.
> 
> +=item B<acpi_conv=BOOLEAN>
> +
> +Include the Windows laptop/slate mode switch device in the virtual
> +firmware ACPI table. False (0) by default.
> +
>  =item B<apic=BOOLEAN>
> 
>  Include information regarding APIC (Advanced Programmable Interrupt
> diff --git a/tools/firmware/hvmloader/util.c
> b/tools/firmware/hvmloader/util.c
> index 03cfb79..9429575 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -947,6 +947,8 @@ void hvmloader_acpi_build_tables(struct acpi_config
> *config,
>          config->table_flags |= ACPI_HAS_SSDT_S3;
>      if ( !strncmp(xenstore_read("platform/acpi_s4", "1"), "1", 1)  )
>          config->table_flags |= ACPI_HAS_SSDT_S4;
> +    if ( !strncmp(xenstore_read("platform/acpi_conv", "1"), "1", 1)  )
> +        config->table_flags |= ACPI_HAS_SSDT_CONV;
> 

I just noticed this is defaulting the wrong way if the toolstack fails to write 
the xenstore key. I'll send v2.

  Paul

>      config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC |
>                              ACPI_HAS_WAET | ACPI_HAS_PMTIMER |
> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
> index 6d8445d..3b61304 100644
> --- a/tools/libacpi/Makefile
> +++ b/tools/libacpi/Makefile
> @@ -25,7 +25,7 @@ C_SRC-$(CONFIG_X86) = dsdt_anycpu.c dsdt_15cpu.c
> dsdt_anycpu_qemu_xen.c dsdt_pvh
>  C_SRC-$(CONFIG_ARM_64) = dsdt_anycpu_arm.c
>  DSDT_FILES ?= $(C_SRC-y)
>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, $(DSDT_FILES))
> -H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h
> ssdt_tpm.h)
> +H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h
> ssdt_tpm.h ssdt_conv.h)
> 
>  MKDSDT_CFLAGS-$(CONFIG_ARM_64) = -DCONFIG_ARM_64
>  MKDSDT_CFLAGS-$(CONFIG_X86) = -DCONFIG_X86
> @@ -89,7 +89,7 @@ iasl:
>       @echo
>       @exit 1
> 
> -build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h
> +build.o: ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h ssdt_conv.h
> 
>  acpi.a: $(OBJS)
>       $(AR) rc $@ $(OBJS)
> diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
> index a02ffbf..698f39a 100644
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -20,6 +20,7 @@
>  #include "ssdt_s4.h"
>  #include "ssdt_tpm.h"
>  #include "ssdt_pm.h"
> +#include "ssdt_conv.h"
>  #include <xen/hvm/hvm_info_table.h>
>  #include <xen/hvm/hvm_xs_strings.h>
>  #include <xen/hvm/params.h>
> @@ -406,6 +407,16 @@ static int construct_secondary_tables(struct
> acpi_ctxt *ctxt,
>          printf("S4 disabled\n");
>      }
> 
> +    if ( config->table_flags & ACPI_HAS_SSDT_CONV )
> +    {
> +        ssdt = ctxt->mem_ops.alloc(ctxt, sizeof(ssdt_conv), 16);
> +        if (!ssdt) return -1;
> +        memcpy(ssdt, ssdt_conv, sizeof(ssdt_conv));
> +        table_ptrs[nr_tables++] = ctxt->mem_ops.v2p(ctxt, ssdt);
> +    } else {
> +        printf("CONV disabled\n");
> +    }
> +
>      /* TPM TCPA and SSDT. */
>      if ( (config->table_flags & ACPI_HAS_TCPA) &&
>           (config->tis_hdr[0] == tis_signature[0]) &&
> diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
> index 67bd67f..e5b91aa 100644
> --- a/tools/libacpi/libacpi.h
> +++ b/tools/libacpi/libacpi.h
> @@ -35,6 +35,7 @@
>  #define ACPI_HAS_VGA         (1<<12)
>  #define ACPI_HAS_8042        (1<<13)
>  #define ACPI_HAS_CMOS_RTC    (1<<14)
> +#define ACPI_HAS_SSDT_CONV   (1<<15)
> 
>  struct xen_vmemrange;
>  struct acpi_numa {
> diff --git a/tools/libacpi/ssdt_conv.asl b/tools/libacpi/ssdt_conv.asl
> new file mode 100644
> index 0000000..d2eb744
> --- /dev/null
> +++ b/tools/libacpi/ssdt_conv.asl
> @@ -0,0 +1,38 @@
> +/*
> + * ssdt_conv.asl
> + *
> + * Copyright (c) 2017  Citrix Systems, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +/*
> + * Windows laptop/slate mode device
> + *
> + * See https://msdn.microsoft.com/en-
> us/windows/hardware/commercialize/design/device-
> experiences/continuum#method-2----use-the-injection-interface
> + */
> +
> +DefinitionBlock ("SSDT_CONV.aml", "SSDT", 2, "Xen", "HVM", 0)
> +{
> +    Device (CONV) {
> +        Method (_HID, 0x0, NotSerialized) {
> +            Return("ID9001")
> +        }
> +        Name (_CID, "PNP0C60")
> +    }
> +}
> +
> +/*
> + * Local variables:
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 25389e1..cb41b08 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -313,6 +313,7 @@ int libxl__domain_build_info_setdefault(libxl__gc
> *gc,
>          libxl_defbool_setdefault(&b_info->u.hvm.acpi,               true);
>          libxl_defbool_setdefault(&b_info->u.hvm.acpi_s3,            true);
>          libxl_defbool_setdefault(&b_info->u.hvm.acpi_s4,            true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.acpi_conv,          false);
>          libxl_defbool_setdefault(&b_info->u.hvm.nx,                 true);
>          libxl_defbool_setdefault(&b_info->u.hvm.viridian,           false);
>          libxl_defbool_setdefault(&b_info->u.hvm.hpet,               true);
> @@ -458,6 +459,8 @@ int libxl__domain_build(libxl__gc *gc,
>          localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
>          localents[i++] = "platform/acpi_s4";
>          localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
> +        localents[i++] = "platform/acpi_conv";
> +        localents[i++] = libxl_defbool_val(info->u.hvm.acpi_conv) ? "1" : 
> "0";
>          if (info->u.hvm.mmio_hole_memkb) {
>              uint64_t max_ram_below_4g =
>                  (1ULL << 32) - (info->u.hvm.mmio_hole_memkb << 10);
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 2475a4d..7e9f781 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -507,6 +507,7 @@ libxl_domain_build_info =
> Struct("domain_build_info",[
>                                         ("acpi",             libxl_defbool),
>                                         ("acpi_s3",          libxl_defbool),
>                                         ("acpi_s4",          libxl_defbool),
> +                                       ("acpi_conv",        libxl_defbool),
>                                         ("nx",               libxl_defbool),
>                                         ("viridian",         libxl_defbool),
>                                         ("viridian_enable",  libxl_bitmap),
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index b72f990..b8eb546 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -1065,6 +1065,7 @@ void parse_config_data(const char *config_source,
>          xlu_cfg_get_defbool(config, "apic", &b_info->u.hvm.apic, 0);
>          xlu_cfg_get_defbool(config, "acpi_s3", &b_info->u.hvm.acpi_s3, 0);
>          xlu_cfg_get_defbool(config, "acpi_s4", &b_info->u.hvm.acpi_s4, 0);
> +        xlu_cfg_get_defbool(config, "acpi_conv", &b_info->u.hvm.acpi_conv,
> 0);
>          xlu_cfg_get_defbool(config, "nx", &b_info->u.hvm.nx, 0);
>          xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
>          xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 
> 0);
> --
> 2.1.4


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