[PATCH v4 00/16] Xen ARM DomU ACPI support

[PATCH v4 00/16] Xen ARM DomU ACPI support

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

The design of this feature is described as below.
Firstly, the toolstack (libxl) generates the ACPI tables according the
number of vcpus and gic controller.

Then, it copies these ACPI tables to DomU non-RAM memory map space and
passes them to UEFI firmware through the "ARM multiboot" protocol.

At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
and installs these tables like the usual way and passes both ACPI and DT
information to the Xen DomU.

Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
since it's enough now.

This has been tested using guest kernel with the Dom0 ACPI support
patches which could be fetched from linux master or:
https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen

The UEFI binary could be fetched from or built from edk2 master branch:
http://people.linaro.org/~shannon.zhao/DomU_ACPI/XEN_EFI.fd

This series can be fetched from:
https://git.linaro.org/people/shannon.zhao/xen.git  domu_acpi_v4

Changes since v3:
* use goto style error handle
* unify configuration option for ACPI
* use extended_checksum instead of checksum in RSDP table
* only require iasl on arm64
* count acpi tables size for maxmem

Changes since v2:
* return error for 32bit domain with acpi enabled
* include actypes.h to reuse the definitions
* rename libxl_arm_acpi.h to libxl_arm.h
* use ACPI_MADT_ENABLED
* rebased on top of Boris's ACPI branch to reuse mk_dsdt.c

Changes since v1:
* move ACPI tables generation codes to a new file
* use static asl file to generate DSDT table and include processor
  device objects
* assign a non-RAM map for ACPI blob
* use existing ACPI table definitions under xen/include/acpi/
* add a configuration for user to enable/disable ACPI generation
* calculate the ACPI table checksum

Shannon Zhao (16):
  tools/libxl: Add an unified configuration option for ACPI
  libxl/arm: prepare for constructing ACPI tables
  libxl/arm: Generate static ACPI DSDT table
  libxl/arm: Estimate the size of ACPI tables
  libxl/arm: Construct ACPI RSDP table
  libxl/arm: Construct ACPI XSDT table
  libxl/arm: Construct ACPI GTDT table
  libxl/arm: Factor MPIDR computing codes out as a helper
  libxl/arm: Construct ACPI MADT table
  libxl/arm: Construct ACPI FADT table
  libxl/arm: Construct ACPI DSDT table
  libxl/arm: Factor finalise_one_memory_node as a gerneric function
  libxl/arm: Add ACPI module
  public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI
  libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ
  libxl/arm: Add the size of ACPI tables to maxmem

 docs/misc/arm/device-tree/acpi.txt |  24 +++
 tools/configure                    |   2 +-
 tools/libacpi/Makefile             |  15 +-
 tools/libacpi/mk_dsdt.c            |  51 ++++--
 tools/libxl/Makefile               |   7 +
 tools/libxl/libxl_arm.c            |  87 +++++++--
 tools/libxl/libxl_arm.h            |  55 ++++++
 tools/libxl/libxl_arm_acpi.c       | 365 +++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_create.c         |   9 +-
 tools/libxl/libxl_dm.c             |   6 +-
 tools/libxl/libxl_types.idl        |   4 +
 tools/libxl/xl_cmdimpl.c           |   2 +-
 xen/arch/arm/domain_build.c        |   8 +-
 xen/include/public/arch-arm.h      |   7 +
 xen/include/public/hvm/params.h    |   4 +
 15 files changed, 612 insertions(+), 34 deletions(-)
 create mode 100644 docs/misc/arm/device-tree/acpi.txt
 create mode 100644 tools/libxl/libxl_arm.h
 create mode 100644 tools/libxl/libxl_arm_acpi.c

-- 
2.0.4



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

[PATCH v4 01/16] tools/libxl: Add an unified configuration option for ACPI

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Since the existing configuration option "u.hvm.acpi" is x86 specific and
we want to reuse it on ARM as well, add a unified option "acpi" for
x86 and ARM, and for ARM it's disabled by default.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_create.c  | 9 ++++++++-
 tools/libxl/libxl_dm.c      | 6 ++++--
 tools/libxl/libxl_types.idl | 4 ++++
 tools/libxl/xl_cmdimpl.c    | 2 +-
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 08822e3..3043b1f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -215,6 +215,12 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
     if (!b_info->event_channels)
         b_info->event_channels = 1023;
 
+#if defined(__arm__) || defined(__aarch64__)
+    libxl_defbool_setdefault(&b_info->acpi, false);
+#else
+    libxl_defbool_setdefault(&b_info->acpi, true);
+#endif
+
     switch (b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
         if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
@@ -454,7 +460,8 @@ int libxl__domain_build(libxl__gc *gc,
         localents = libxl__calloc(gc, 9, sizeof(char *));
         i = 0;
         localents[i++] = "platform/acpi";
-        localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
+        localents[i++] = (libxl_defbool_val(info->acpi) &&
+                         libxl_defbool_val(info->u.hvm.acpi)) ? "1" : "0";
         localents[i++] = "platform/acpi_s3";
         localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
         localents[i++] = "platform/acpi_s4";
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index de16a59..12e4084 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -583,7 +583,8 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
         if (b_info->u.hvm.soundhw) {
             flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL);
         }
-        if (libxl_defbool_val(b_info->u.hvm.acpi)) {
+        if (libxl_defbool_val(b_info->acpi) &&
+            libxl_defbool_val(b_info->u.hvm.acpi)) {
             flexarray_append(dm_args, "-acpi");
         }
         if (b_info->max_vcpus > 1) {
@@ -1204,7 +1205,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
         if (b_info->u.hvm.soundhw) {
             flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL);
         }
-        if (!libxl_defbool_val(b_info->u.hvm.acpi)) {
+        if (!(libxl_defbool_val(b_info->acpi) &&
+             libxl_defbool_val(b_info->u.hvm.acpi))) {
             flexarray_append(dm_args, "-no-acpi");
         }
         if (b_info->max_vcpus > 1) {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 98bfc3a..a02446f 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -494,11 +494,15 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # Note that the partial device tree should avoid to use the phandle
     # 65000 which is reserved by the toolstack.
     ("device_tree",      string),
+    ("acpi",             libxl_defbool),
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
                                        ("pae",              libxl_defbool),
                                        ("apic",             libxl_defbool),
+                                       # The following acpi field is deprecated.
+                                       # Please use the unified acpi field above
+                                       # which works for both x86 and ARM.
                                        ("acpi",             libxl_defbool),
                                        ("acpi_s3",          libxl_defbool),
                                        ("acpi_s4",          libxl_defbool),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1d06598..be17702 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1547,6 +1547,7 @@ static void parse_config_data(const char *config_source,
     b_info->cmdline = parse_cmdline(config);
 
     xlu_cfg_get_defbool(config, "driver_domain", &c_info->driver_domain, 0);
+    xlu_cfg_get_defbool(config, "acpi", &b_info->acpi, 0);
 
     switch(b_info->type) {
     case LIBXL_DOMAIN_TYPE_HVM:
@@ -1576,7 +1577,6 @@ static void parse_config_data(const char *config_source,
 
         xlu_cfg_get_defbool(config, "pae", &b_info->u.hvm.pae, 0);
         xlu_cfg_get_defbool(config, "apic", &b_info->u.hvm.apic, 0);
-        xlu_cfg_get_defbool(config, "acpi", &b_info->u.hvm.acpi, 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, "nx", &b_info->u.hvm.nx, 0);
-- 
2.0.4



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

[PATCH v4 02/16] libxl/arm: prepare for constructing ACPI tables

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

It only constructs the ACPI tables for 64-bit ARM DomU when user enables
acpi because 32-bit DomU doesn't support ACPI.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/Makefile          |  4 +++
 tools/libxl/libxl_arm.c       | 23 +++++++++++++++-
 tools/libxl/libxl_arm.h       | 44 +++++++++++++++++++++++++++++++
 tools/libxl/libxl_arm_acpi.c  | 61 +++++++++++++++++++++++++++++++++++++++++++
 xen/include/public/arch-arm.h |  4 +++
 5 files changed, 135 insertions(+), 1 deletion(-)
 create mode 100644 tools/libxl/libxl_arm.h
 create mode 100644 tools/libxl/libxl_arm_acpi.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index a148374..6139bed 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -90,6 +90,10 @@ acpi:
 
 LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o libxl_x86_acpi.o
 LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
+LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o
+
+libxl_arm_acpi.o: libxl_arm_acpi.c
+	$(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
 
 ifeq ($(CONFIG_NetBSD),y)
 LIBXL_OBJS-y += libxl_netbsd.o
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index bd3d611..8c7fc09 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -1,6 +1,7 @@
 #include "libxl_internal.h"
 #include "libxl_arch.h"
 #include "libxl_libfdt_compat.h"
+#include "libxl_arm.h"
 
 #include <xc_dom.h>
 #include <stdbool.h>
@@ -885,8 +886,28 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
                                            libxl__domain_build_state *state,
                                            struct xc_dom_image *dom)
 {
+    int rc;
+
     assert(info->type == LIBXL_DOMAIN_TYPE_PV);
-    return libxl__prepare_dtb(gc, info, state, dom);
+    rc = libxl__prepare_dtb(gc, info, state, dom);
+    if (rc) goto out;
+
+    if (!libxl_defbool_val(info->acpi)) {
+        LOG(DEBUG, "Generating ACPI tables is disabled by user.");
+        rc = 0;
+        goto out;
+    }
+
+    if (strcmp(dom->guest_type, "xen-3.0-aarch64")) {
+        LOG(ERROR, "Can not enable libxl option 'acpi' for %s", dom->guest_type);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = libxl__prepare_acpi(gc, info, state, dom);
+
+out:
+    return rc;
 }
 
 static void finalise_one_memory_node(libxl__gc *gc, void *fdt,
diff --git a/tools/libxl/libxl_arm.h b/tools/libxl/libxl_arm.h
new file mode 100644
index 0000000..fe1c05f
--- /dev/null
+++ b/tools/libxl/libxl_arm.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2016      Linaro Ltd.
+ *
+ * Author: Shannon Zhao <shannon.zhao@linaro.org>
+ *
+ * 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.
+ */
+
+#include "libxl_internal.h"
+#include "libxl_arch.h"
+
+#include <xc_dom.h>
+
+#if defined(__aarch64__)
+_hidden
+int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
+                        libxl__domain_build_state *state,
+                        struct xc_dom_image *dom);
+#else
+_hidden
+static inline int libxl__prepare_acpi(libxl__gc *gc,
+                                      libxl_domain_build_info *info,
+                                      libxl__domain_build_state *state,
+                                      struct xc_dom_image *dom)
+{
+    return 0;
+}
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
new file mode 100644
index 0000000..ec6cf08
--- /dev/null
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -0,0 +1,61 @@
+/*
+ * ARM DomU ACPI generation
+ *
+ * Copyright (C) 2016      Linaro Ltd.
+ *
+ * Author: Shannon Zhao <shannon.zhao@linaro.org>
+ *
+ * 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.
+ */
+
+#include "libxl_arm.h"
+
+#include <stdint.h>
+
+typedef uint8_t u8;
+typedef uint16_t u16;
+typedef uint32_t u32;
+typedef uint64_t u64;
+
+#include <acpi/acconfig.h>
+#include <acpi/actbl.h>
+
+int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
+                        libxl__domain_build_state *state,
+                        struct xc_dom_image *dom)
+{
+    const libxl_version_info *vers;
+    int rc = 0;
+
+    vers = libxl_get_version_info(CTX);
+    if (vers == NULL) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    LOG(DEBUG, "constructing ACPI tables for Xen version %d.%d guest",
+        vers->xen_version_major, vers->xen_version_minor);
+
+    dom->acpi_modules[0].data = NULL;
+    dom->acpi_modules[0].length = 0;
+    dom->acpi_modules[0].guest_addr_out = GUEST_ACPI_BASE;
+
+out:
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 870bc3b..0afd654 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -406,6 +406,10 @@ typedef uint64_t xen_callback_t;
 #define GUEST_GICV3_GICR0_BASE     0x03020000ULL    /* vCPU0 - vCPU127 */
 #define GUEST_GICV3_GICR0_SIZE     0x01000000ULL
 
+/* ACPI tables physical address */
+#define GUEST_ACPI_BASE 0x20000000ULL
+#define GUEST_ACPI_SIZE 0x00200000ULL
+
 /*
  * 16MB == 4096 pages reserved for guest to use as a region to map its
  * grant table in.
-- 
2.0.4



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

[PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

It uses static DSDT table like the way x86 uses. Currently the DSDT
table only contains processor device objects and it generates the
maximal objects which so far is 128.

Also only check iasl for aarch64 in configure since ACPI on ARM32 is not
supported.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/configure               |  2 +-
 tools/libacpi/Makefile        | 15 ++++++++++++-
 tools/libacpi/mk_dsdt.c       | 51 ++++++++++++++++++++++++++++++++-----------
 tools/libxl/Makefile          |  5 ++++-
 tools/libxl/libxl_arm_acpi.c  |  5 +++++
 xen/include/public/arch-arm.h |  3 +++
 6 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/tools/configure b/tools/configure
index 5b5dcce..48239c0 100755
--- a/tools/configure
+++ b/tools/configure
@@ -7458,7 +7458,7 @@ then
     as_fn_error $? "Unable to find xgettext, please install xgettext" "$LINENO" 5
 fi
 case "$host_cpu" in
-i[3456]86|x86_64)
+i[3456]86|x86_64|aarch64)
     # Extract the first word of "iasl", so it can be a program name with args.
 set dummy iasl; ac_word=$2
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
index d741ac5..7f50a33 100644
--- a/tools/libacpi/Makefile
+++ b/tools/libacpi/Makefile
@@ -19,6 +19,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
 
 # Sources to be generated
 C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c  dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
+C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c
 H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h)
 
 vpath iasl $(PATH)
@@ -32,7 +33,7 @@ $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
 	cd $(CURDIR)
 
 $(MK_DSDT): mk_dsdt.c
-	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
+	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -o $@ mk_dsdt.c
 
 $(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.asl: dsdt.asl dsdt_acpi_info.asl $(MK_DSDT)
 	awk 'NR > 1 {print s} {s=$$0}' $< > $@
@@ -62,6 +63,18 @@ $(ACPI_BUILD_DIR)/dsdt_pvh.c: iasl $(ACPI_BUILD_DIR)/dsdt_pvh.asl
 	echo "int dsdt_pvh_len=sizeof(dsdt_pvh);" >>$@
 	rm -f $(ACPI_BUILD_DIR)/$*.aml $(ACPI_BUILD_DIR)/$*.hex
 
+$(ACPI_BUILD_DIR)/dsdt_anycpu_arm.asl: $(MK_DSDT)
+	printf "DefinitionBlock (\"DSDT.aml\", \"DSDT\", 3, \"XenARM\", \"Xen DSDT\", 1)\n{" > $@
+	$(MK_DSDT) --debug=$(debug) --arch arm >> $@
+
+$(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c: iasl $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.asl
+	cd $(ACPI_BUILD_DIR)
+	iasl -vs -p $* -tc $(ACPI_BUILD_DIR)/$*.asl
+	sed -e 's/AmlCode/$*/g' $*.hex >$@
+	echo "int $*_len=sizeof($*);" >>$@
+	rm -f $*.aml $*.hex
+	cd $(CURDIR)
+
 iasl:
 	@echo
 	@echo "ACPI ASL compiler (iasl) is needed"
diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
index 7d76784..f3ab28f 100644
--- a/tools/libacpi/mk_dsdt.c
+++ b/tools/libacpi/mk_dsdt.c
@@ -18,6 +18,7 @@
 #include <stdlib.h>
 #include <stdbool.h>
 #include <xen/hvm/hvm_info_table.h>
+#include <xen/arch-arm.h>
 
 static unsigned int indent_level;
 static bool debug = false;
@@ -99,6 +100,7 @@ static struct option options[] = {
     { "dm-version", 1, 0, 'q' },
     { "debug", 1, 0, 'd' },
     { "no-dm", 0, 0, 'n' },
+    { "arch", 1, 0, 'a' },
     { 0, 0, 0, 0 }
 };
 
@@ -106,7 +108,7 @@ int main(int argc, char **argv)
 {
     unsigned int slot, dev, intx, link, cpu, max_cpus = HVM_MAX_VCPUS;
     dm_version dm_version = QEMU_XEN_TRADITIONAL;
-    bool no_dm = 0;
+    bool no_dm = 0, arch_is_arm = false;
 
     for ( ; ; )
     {
@@ -145,6 +147,10 @@ int main(int argc, char **argv)
         case 'n':
             no_dm = 1;
             break;
+        case 'a':
+            if (strcmp(optarg, "arm") == 0)
+                arch_is_arm = true;
+            break;
         case 'd':
             if (*optarg == 'y')
                 debug = true;
@@ -154,6 +160,9 @@ int main(int argc, char **argv)
         }
     }
 
+    if (arch_is_arm)
+        max_cpus = GUEST_MAX_VCPUS;
+
     /**** DSDT DefinitionBlock start ****/
     /* (we append to existing DSDT definition block) */
     indent_level++;
@@ -161,19 +170,21 @@ int main(int argc, char **argv)
     /**** Processor start ****/
     push_block("Scope", "\\_SB");
 
-    /* MADT checksum */
-    stmt("OperationRegion", "MSUM, SystemMemory, \\_SB.MSUA, 1");
-    push_block("Field", "MSUM, ByteAcc, NoLock, Preserve");
-    indent(); printf("MSU, 8\n");
-    pop_block();
+    if (!arch_is_arm) {
+        /* MADT checksum */
+        stmt("OperationRegion", "MSUM, SystemMemory, \\_SB.MSUA, 1");
+        push_block("Field", "MSUM, ByteAcc, NoLock, Preserve");
+        indent(); printf("MSU, 8\n");
+        pop_block();
 
-    /* Processor object helpers. */
-    push_block("Method", "PMAT, 2");
-    push_block("If", "LLess(Arg0, NCPU)");
-    stmt("Return", "ToBuffer(Arg1)");
-    pop_block();
-    stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
-    pop_block();
+        /* Processor object helpers. */
+        push_block("Method", "PMAT, 2");
+        push_block("If", "LLess(Arg0, NCPU)");
+        stmt("Return", "ToBuffer(Arg1)");
+        pop_block();
+        stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
+        pop_block();
+    }
 
     /* Define processor objects and control methods. */
     for ( cpu = 0; cpu < max_cpus; cpu++)
@@ -182,6 +193,12 @@ int main(int argc, char **argv)
 
         stmt("Name", "_HID, \"ACPI0007\"");
 
+        if (arch_is_arm) {
+            stmt("Name", "_UID, %d", cpu);
+            pop_block();
+            continue;
+        }
+
         /* Name this processor's MADT LAPIC descriptor. */
         stmt("OperationRegion", 
              "MATR, SystemMemory, Add(\\_SB.MAPA, %d), 8", cpu*8);
@@ -220,6 +237,14 @@ int main(int argc, char **argv)
         pop_block();
     }
 
+    if (arch_is_arm) {
+        pop_block();
+        /**** Processor end ****/
+        pop_block();
+        /**** DSDT DefinitionBlock end ****/
+        return 0;
+    }
+
     /* Operation Region 'PRST': bitmask of online CPUs. */
     stmt("OperationRegion", "PRST, SystemIO, 0xaf00, 32");
     push_block("Field", "PRST, ByteAcc, NoLock, Preserve");
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 6139bed..ce356d0 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -90,7 +90,10 @@ acpi:
 
 LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o libxl_x86_acpi.o
 LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
-LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o
+LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o dsdt_anycpu_arm.o
+
+dsdt_anycpu_arm.c:
+	$(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(shell pwd)
 
 libxl_arm_acpi.o: libxl_arm_acpi.c
 	$(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index ec6cf08..087d028 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -28,6 +28,11 @@ typedef uint64_t u64;
 #include <acpi/acconfig.h>
 #include <acpi/actbl.h>
 
+_hidden
+extern const unsigned char dsdt_anycpu_arm[];
+_hidden
+extern const int dsdt_anycpu_arm_len;
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 0afd654..008a2a0 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -435,6 +435,9 @@ typedef uint64_t xen_callback_t;
 #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
 #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
 
+/* Current supported guest VCPUs */
+#define GUEST_MAX_VCPUS 128
+
 /* Interrupts */
 #define GUEST_TIMER_VIRT_PPI    27
 #define GUEST_TIMER_PHYS_S_PPI  29
-- 
2.0.4



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

[PATCH v4 04/16] libxl/arm: Estimate the size of ACPI tables

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Estimate the size of ACPI tables and reserve a memory map space for ACPI
tables.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 087d028..6be9eb0 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -33,12 +33,95 @@ extern const unsigned char dsdt_anycpu_arm[];
 _hidden
 extern const int dsdt_anycpu_arm_len;
 
+enum {
+    RSDP,
+    XSDT,
+    GTDT,
+    MADT,
+    FADT,
+    DSDT,
+    NUMS,
+};
+
+struct acpitable {
+    uint64_t addr;
+    size_t size;
+};
+
+static int libxl__estimate_acpi_size(libxl__gc *gc,
+                                     libxl_domain_build_info *info,
+                                     struct xc_dom_image *dom,
+                                     xc_domain_configuration_t *xc_config,
+                                     struct acpitable acpitables[])
+{
+    uint64_t size;
+    int rc = 0;
+
+    acpitables[RSDP].addr = GUEST_ACPI_BASE;
+    acpitables[RSDP].size = sizeof(struct acpi_table_rsdp);
+    dom->acpi_modules[0].length += ROUNDUP(acpitables[RSDP].size, 3);
+
+    acpitables[XSDT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
+    /*
+     * Currently only 3 tables(GTDT, FADT, MADT) are pointed by XSDT. Alloc
+     * entries for them.
+     */
+    acpitables[XSDT].size = sizeof(struct acpi_table_xsdt) +
+                            sizeof(uint64_t) * 2;
+    dom->acpi_modules[0].length += ROUNDUP(acpitables[XSDT].size, 3);
+
+    acpitables[GTDT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
+    acpitables[GTDT].size = sizeof(struct acpi_table_gtdt);
+    dom->acpi_modules[0].length += ROUNDUP(acpitables[GTDT].size, 3);
+
+    acpitables[MADT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
+
+    switch (xc_config->gic_version) {
+    case XEN_DOMCTL_CONFIG_GIC_V2:
+        size = sizeof(struct acpi_table_madt) +
+               sizeof(struct acpi_madt_generic_interrupt) * info->max_vcpus +
+               sizeof(struct acpi_madt_generic_distributor);
+        break;
+    case XEN_DOMCTL_CONFIG_GIC_V3:
+        size = sizeof(struct acpi_table_madt) +
+               sizeof(struct acpi_madt_generic_interrupt) * info->max_vcpus +
+               sizeof(struct acpi_madt_generic_distributor) +
+               sizeof(struct acpi_madt_generic_redistributor);
+        break;
+    default:
+        LOG(ERROR, "Unknown GIC version");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    acpitables[MADT].size = size;
+    dom->acpi_modules[0].length += ROUNDUP(acpitables[MADT].size, 3);
+
+    acpitables[FADT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
+    acpitables[FADT].size = sizeof(struct acpi_table_fadt);
+    dom->acpi_modules[0].length += ROUNDUP(acpitables[FADT].size, 3);
+
+    acpitables[DSDT].addr = GUEST_ACPI_BASE + dom->acpi_modules[0].length;
+    acpitables[DSDT].size = dsdt_anycpu_arm_len;
+    dom->acpi_modules[0].length += ROUNDUP(acpitables[DSDT].size, 3);
+
+    assert(dom->acpi_modules[0].length <= GUEST_ACPI_SIZE);
+    dom->acpi_modules[0].data = libxl__zalloc(gc, dom->acpi_modules[0].length);
+
+out:
+    return rc;
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
 {
     const libxl_version_info *vers;
     int rc = 0;
+    struct acpitable acpitables[NUMS];
+
+    /* convenience aliases */
+    xc_domain_configuration_t *xc_config = &state->config;
 
     vers = libxl_get_version_info(CTX);
     if (vers == NULL) {
@@ -53,6 +136,8 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
     dom->acpi_modules[0].length = 0;
     dom->acpi_modules[0].guest_addr_out = GUEST_ACPI_BASE;
 
+    rc = libxl__estimate_acpi_size(gc, info, dom, xc_config, acpitables);
+
 out:
     return rc;
 }
-- 
2.0.4



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

[PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Construct ACPI RSDP table and add a helper to calculate the ACPI table
checksum.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 6be9eb0..9432e44 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
 _hidden
 extern const int dsdt_anycpu_arm_len;
 
+#define ACPI_BUILD_APPNAME6 "XenARM"
+#define ACPI_BUILD_APPNAME4 "Xen "
+
 enum {
     RSDP,
     XSDT,
@@ -112,6 +115,37 @@ out:
     return rc;
 }
 
+static void calculate_checksum(void *table, uint32_t checksum_offset,
+                               uint32_t length)
+{
+    uint8_t *p, sum = 0;
+
+    p = table;
+    p[checksum_offset] = 0;
+
+    while ( length-- )
+        sum = sum + *p++;
+
+    p = table;
+    p[checksum_offset] = -sum;
+}
+
+static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
+                           struct acpitable acpitables[])
+{
+    uint64_t offset = acpitables[RSDP].addr - GUEST_ACPI_BASE;
+    struct acpi_table_rsdp *rsdp = (void *)dom->acpi_modules[0].data + offset;
+
+    memcpy(rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
+    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
+    rsdp->length = acpitables[RSDP].size;
+    rsdp->revision = 0x02;
+    rsdp->xsdt_physical_address = acpitables[XSDT].addr;
+    calculate_checksum(rsdp,
+                       offsetof(struct acpi_table_rsdp, extended_checksum),
+                       acpitables[RSDP].size);
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -137,6 +171,10 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
     dom->acpi_modules[0].guest_addr_out = GUEST_ACPI_BASE;
 
     rc = libxl__estimate_acpi_size(gc, info, dom, xc_config, acpitables);
+    if (rc)
+        goto out;
+
+    make_acpi_rsdp(gc, dom, acpitables);
 
 out:
     return rc;
-- 
2.0.4



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

[PATCH v4 06/16] libxl/arm: Construct ACPI XSDT table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 9432e44..8cd1d9b 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -146,6 +146,35 @@ static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
                        acpitables[RSDP].size);
 }
 
+static void make_acpi_header(struct acpi_table_header *h, const char *sig,
+                             int len, uint8_t rev)
+{
+    memcpy(h->signature, sig, 4);
+    h->length = len;
+    h->revision = rev;
+    memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
+    memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
+    memcpy(h->oem_table_id + 4, sig, 4);
+    h->oem_revision = 1;
+    memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
+    h->asl_compiler_revision = 1;
+    h->checksum = 0;
+}
+
+static void make_acpi_xsdt(libxl__gc *gc, struct xc_dom_image *dom,
+                           struct acpitable acpitables[])
+{
+    uint64_t offset = acpitables[XSDT].addr - GUEST_ACPI_BASE;
+    struct acpi_table_xsdt *xsdt = (void *)dom->acpi_modules[0].data + offset;
+
+    xsdt->table_offset_entry[0] = acpitables[MADT].addr;
+    xsdt->table_offset_entry[1] = acpitables[GTDT].addr;
+    xsdt->table_offset_entry[2] = acpitables[FADT].addr;
+    make_acpi_header(&xsdt->header, "XSDT", acpitables[XSDT].size, 1);
+    calculate_checksum(xsdt, offsetof(struct acpi_table_header, checksum),
+                       acpitables[XSDT].size);
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -175,6 +204,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
         goto out;
 
     make_acpi_rsdp(gc, dom, acpitables);
+    make_acpi_xsdt(gc, dom, acpitables);
 
 out:
     return rc;
-- 
2.0.4



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

[PATCH v4 07/16] libxl/arm: Construct ACPI GTDT table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Construct GTDT table with the interrupt information of timers.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 8cd1d9b..28fb6fe 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -24,10 +24,18 @@ typedef uint8_t u8;
 typedef uint16_t u16;
 typedef uint32_t u32;
 typedef uint64_t u64;
+typedef int64_t s64;
 
 #include <acpi/acconfig.h>
 #include <acpi/actbl.h>
 
+#include <asm-generic/bitsperlong.h>
+#define ACPI_MACHINE_WIDTH __BITS_PER_LONG
+#define COMPILER_DEPENDENT_INT64 int64_t
+#define COMPILER_DEPENDENT_UINT64 uint64_t
+
+#include <acpi/actypes.h>
+
 _hidden
 extern const unsigned char dsdt_anycpu_arm[];
 _hidden
@@ -175,6 +183,26 @@ static void make_acpi_xsdt(libxl__gc *gc, struct xc_dom_image *dom,
                        acpitables[XSDT].size);
 }
 
+static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom,
+                           struct acpitable acpitables[])
+{
+    uint64_t offset = acpitables[GTDT].addr - GUEST_ACPI_BASE;
+    struct acpi_table_gtdt *gtdt = (void *)dom->acpi_modules[0].data + offset;
+
+    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
+    gtdt->non_secure_el1_flags =
+                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
+                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
+    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
+    gtdt->virtual_timer_flags =
+                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
+                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
+
+    make_acpi_header(&gtdt->header, "GTDT", acpitables[GTDT].size, 2);
+    calculate_checksum(gtdt, offsetof(struct acpi_table_header, checksum),
+                       acpitables[GTDT].size);
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -205,6 +233,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
 
     make_acpi_rsdp(gc, dom, acpitables);
     make_acpi_xsdt(gc, dom, acpitables);
+    make_acpi_gtdt(gc, dom, acpitables);
 
 out:
     return rc;
-- 
2.0.4



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

[PATCH v4 08/16] libxl/arm: Factor MPIDR computing codes out as a helper

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Factor MPIDR computing codes out as a helper, so it could be shared
between DT and ACPI.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm.c |  8 +-------
 tools/libxl/libxl_arm.h | 11 +++++++++++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 8c7fc09..fa0497c 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -309,13 +309,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
     for (i = 0; i < nr_cpus; i++) {
         const char *name;
 
-        /*
-         * According to ARM CPUs bindings, the reg field should match
-         * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
-         * constructing the reg value of the guest at the moment, for it
-         * is enough for the current max vcpu number.
-         */
-        mpidr_aff = (i & 0x0f) | (((i >> 4) & 0xff) << 8);
+        mpidr_aff = libxl__compute_mpdir(i);
         name = GCSPRINTF("cpu@%"PRIx64, mpidr_aff);
 
         res = fdt_begin_node(fdt, name);
diff --git a/tools/libxl/libxl_arm.h b/tools/libxl/libxl_arm.h
index fe1c05f..5c8fbc6 100644
--- a/tools/libxl/libxl_arm.h
+++ b/tools/libxl/libxl_arm.h
@@ -35,6 +35,17 @@ static inline int libxl__prepare_acpi(libxl__gc *gc,
 }
 #endif
 
+static inline uint64_t libxl__compute_mpdir(unsigned int cpuid)
+{
+    /*
+     * According to ARM CPUs bindings, the reg field should match
+     * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
+     * constructing the reg value of the guest at the moment, for it
+     * is enough for the current max vcpu number.
+     */
+    return (cpuid & 0x0f) | (((cpuid >> 4) & 0xff) << 8);
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.0.4



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

[PATCH v4 09/16] libxl/arm: Construct ACPI MADT table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

According to the GIC version, construct the MADT table.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 28fb6fe..75dfcc2 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -203,6 +203,89 @@ static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom,
                        acpitables[GTDT].size);
 }
 
+static void make_acpi_madt_gicc(void *table, int nr_cpus, uint64_t gicc_base)
+{
+    uint32_t i;
+    struct acpi_madt_generic_interrupt *gicc = table;
+
+    for (i = 0; i < nr_cpus; i++) {
+        gicc->header.type = ACPI_MADT_TYPE_GENERIC_INTERRUPT;
+        gicc->header.length = sizeof(*gicc);
+        gicc->base_address = gicc_base;
+        gicc->cpu_interface_number = i;
+        gicc->arm_mpidr = libxl__compute_mpdir(i);
+        gicc->uid = i;
+        gicc->flags = ACPI_MADT_ENABLED;
+        gicc++;
+    }
+}
+
+static void make_acpi_madt_gicd(void *table, uint64_t gicd_base,
+                                uint8_t gic_version)
+{
+    struct acpi_madt_generic_distributor *gicd = table;
+
+    gicd->header.type = ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR;
+    gicd->header.length = sizeof(*gicd);
+    gicd->base_address = gicd_base;
+    /* This version field has no meaning before ACPI 5.1 errata. */
+    gicd->version = gic_version;
+}
+
+static void make_acpi_madt_gicr(void *table, uint64_t gicr_base,
+                                uint64_t gicr_size)
+{
+    struct acpi_madt_generic_redistributor *gicr = table;
+
+    gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
+    gicr->header.length = sizeof(*gicr);
+    gicr->base_address = gicr_base;
+    gicr->length = gicr_size;
+}
+
+static int make_acpi_madt(libxl__gc *gc, struct xc_dom_image *dom, int nr_cpus,
+                          xc_domain_configuration_t *xc_config,
+                          struct acpitable acpitables[])
+{
+    uint64_t offset = acpitables[MADT].addr - GUEST_ACPI_BASE;
+    void *table = dom->acpi_modules[0].data + offset;
+    struct acpi_table_madt *madt = table;
+    int rc = 0;
+
+    switch (xc_config->gic_version) {
+    case XEN_DOMCTL_CONFIG_GIC_V2:
+        table += sizeof(struct acpi_table_madt);
+        make_acpi_madt_gicc(table, nr_cpus, GUEST_GICC_BASE);
+
+        table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
+        make_acpi_madt_gicd(table, GUEST_GICD_BASE, ACPI_MADT_GIC_VERSION_V2);
+        break;
+    case XEN_DOMCTL_CONFIG_GIC_V3:
+        table += sizeof(struct acpi_table_madt);
+        make_acpi_madt_gicc(table, nr_cpus, 0);
+
+        table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
+        make_acpi_madt_gicd(table, GUEST_GICV3_GICD_BASE,
+                            ACPI_MADT_GIC_VERSION_V3);
+
+        table += sizeof(struct acpi_madt_generic_distributor);
+        make_acpi_madt_gicr(table, GUEST_GICV3_GICR0_BASE,
+                            GUEST_GICV3_GICR0_SIZE);
+        break;
+    default:
+        LOG(ERROR, "Unknown GIC version");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    make_acpi_header(&madt->header, "APIC", acpitables[MADT].size, 3);
+    calculate_checksum(madt, offsetof(struct acpi_table_header, checksum),
+                       acpitables[MADT].size);
+
+out:
+    return rc;
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -234,6 +317,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
     make_acpi_rsdp(gc, dom, acpitables);
     make_acpi_xsdt(gc, dom, acpitables);
     make_acpi_gtdt(gc, dom, acpitables);
+    rc = make_acpi_madt(gc, dom, info->max_vcpus, xc_config, acpitables);
 
 out:
     return rc;
-- 
2.0.4



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

[PATCH v4 10/16] libxl/arm: Construct ACPI FADT table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index 75dfcc2..cb1c9df 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -286,6 +286,25 @@ out:
     return rc;
 }
 
+static void make_acpi_fadt(libxl__gc *gc, struct xc_dom_image *dom,
+                           struct acpitable acpitables[])
+{
+    uint64_t offset = acpitables[FADT].addr - GUEST_ACPI_BASE;
+    struct acpi_table_fadt *fadt = (void *)dom->acpi_modules[0].data + offset;
+
+    /* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */
+    fadt->flags = ACPI_FADT_HW_REDUCED;
+    fadt->arm_boot_flags = (ACPI_FADT_PSCI_COMPLIANT) | ACPI_FADT_PSCI_USE_HVC;
+
+    /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
+    fadt->minor_revision = 0x1;
+    fadt->dsdt = acpitables[DSDT].addr;
+
+    make_acpi_header(&fadt->header, "FACP", acpitables[FADT].size, 5);
+    calculate_checksum(fadt, offsetof(struct acpi_table_header, checksum),
+                       acpitables[FADT].size);
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -318,6 +337,10 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
     make_acpi_xsdt(gc, dom, acpitables);
     make_acpi_gtdt(gc, dom, acpitables);
     rc = make_acpi_madt(gc, dom, info->max_vcpus, xc_config, acpitables);
+    if (rc)
+        goto out;
+
+    make_acpi_fadt(gc, dom, acpitables);
 
 out:
     return rc;
-- 
2.0.4



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

[PATCH v4 11/16] libxl/arm: Construct ACPI DSDT table

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Copy the static DSDT table into ACPI blob.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm_acpi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index cb1c9df..3154e0c 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -305,6 +305,15 @@ static void make_acpi_fadt(libxl__gc *gc, struct xc_dom_image *dom,
                        acpitables[FADT].size);
 }
 
+static void make_acpi_dsdt(libxl__gc *gc, struct xc_dom_image *dom,
+                           struct acpitable acpitables[])
+{
+    uint64_t offset = acpitables[DSDT].addr - GUEST_ACPI_BASE;
+    void *dsdt = dom->acpi_modules[0].data + offset;
+
+    memcpy(dsdt, dsdt_anycpu_arm, dsdt_anycpu_arm_len);
+}
+
 int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
                         libxl__domain_build_state *state,
                         struct xc_dom_image *dom)
@@ -341,6 +350,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
         goto out;
 
     make_acpi_fadt(gc, dom, acpitables);
+    make_acpi_dsdt(gc, dom, acpitables);
 
 out:
     return rc;
-- 
2.0.4



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

[PATCH v4 12/16] libxl/arm: Factor finalise_one_memory_node as a gerneric function

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Rename finalise_one_memory_node to finalise_one_node and pass the node
name via function parameter.

This is useful for adding ACPI module which will be added by a later
patch.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index fa0497c..b95fdf5 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -904,11 +904,11 @@ out:
     return rc;
 }
 
-static void finalise_one_memory_node(libxl__gc *gc, void *fdt,
-                                     uint64_t base, uint64_t size)
+static void finalise_one_node(libxl__gc *gc, void *fdt, const char *uname,
+                              uint64_t base, uint64_t size)
 {
     int node, res;
-    const char *name = GCSPRINTF("/memory@%"PRIx64, base);
+    const char *name = GCSPRINTF("%s@%"PRIx64, uname, base);
 
     node = fdt_path_offset(fdt, name);
     assert(node > 0);
@@ -971,7 +971,7 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
     for (i = 0; i < GUEST_RAM_BANKS; i++) {
         const uint64_t size = (uint64_t)dom->rambank_size[i] << XC_PAGE_SHIFT;
 
-        finalise_one_memory_node(gc, fdt, bankbase[i], size);
+        finalise_one_node(gc, fdt, "/memory", bankbase[i], size);
     }
 
     debug_dump_fdt(gc, fdt);
-- 
2.0.4



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

[PATCH v4 13/16] libxl/arm: Add ACPI module

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add the ARM Multiboot module for ACPI, so UEFI or DomU can get the base
address of ACPI tables from it.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 docs/misc/arm/device-tree/acpi.txt | 24 ++++++++++++++++++++++++
 tools/libxl/libxl_arm.c            | 24 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 docs/misc/arm/device-tree/acpi.txt

diff --git a/docs/misc/arm/device-tree/acpi.txt b/docs/misc/arm/device-tree/acpi.txt
new file mode 100644
index 0000000..3e70157
--- /dev/null
+++ b/docs/misc/arm/device-tree/acpi.txt
@@ -0,0 +1,24 @@
+DomU ACPI module
+================================
+
+Xen toolstack passes the domU ACPI tables via a reference in the /chosen node of
+the device tree.
+
+Each node contains the following properties:
+
+- compatible
+
+	"xen,guest-acpi", "multiboot,module"
+
+- reg
+
+	Specifies the physical address and the length of the module.
+	RSDP table is always located at the beginning of this region.
+
+Examples
+========
+
+	module@0x20000000 {
+		compatible = "xen,guest-acpi", "multiboot,module";
+		reg = <0x20000000 0x1234>;
+	};
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index b95fdf5..11a6f6e 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -285,6 +285,25 @@ static int make_chosen_node(libxl__gc *gc, void *fdt, bool ramdisk,
         if (res) return res;
     }
 
+    if (libxl_defbool_val(info->acpi)) {
+        const uint64_t acpi_base = GUEST_ACPI_BASE;
+        const char *name = GCSPRINTF("module@%"PRIx64, acpi_base);
+
+        res = fdt_begin_node(fdt, name);
+        if (res) return res;
+
+        res = fdt_property_compat(gc, fdt, 2, "xen,guest-acpi",
+                                  "multiboot,module");
+        if (res) return res;
+
+        res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
+                                1, 0, 0);
+        if (res) return res;
+
+        res = fdt_end_node(fdt);
+        if (res) return res;
+    }
+
     res = fdt_end_node(fdt);
     if (res) return res;
 
@@ -974,6 +993,11 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
         finalise_one_node(gc, fdt, "/memory", bankbase[i], size);
     }
 
+    if (dom->acpi_modules[0].data) {
+        finalise_one_node(gc, fdt, "/chosen/module", GUEST_ACPI_BASE,
+                          dom->acpi_modules[0].length);
+    }
+
     debug_dump_fdt(gc, fdt);
 
     return 0;
-- 
2.0.4



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

[PATCH v4 14/16] public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
them in evtchn_fixup().

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 xen/arch/arm/domain_build.c     | 8 +++++---
 xen/include/public/hvm/params.h | 4 ++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 60db9e4..94cd3ce 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
            d->arch.evtchn_irq);
 
     /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
-    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
-    val |= (2 << 8); /* Active-low level-sensitive  */
-    val |= d->arch.evtchn_irq & 0xff;
+    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
+    /* Active-low level-sensitive  */
+    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
+            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
+    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
     d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
 
     /*
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index f7338a3..8a0327d 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -30,6 +30,7 @@
  */
 
 #define HVM_PARAM_CALLBACK_IRQ 0
+#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
 /*
  * How should CPU0 event-channel notifications be delivered?
  *
@@ -66,6 +67,9 @@
  * This is only used by ARM/ARM64 and masking/eoi the interrupt associated to
  * the notification is handled by the interrupt controller.
  */
+#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
+#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
+#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
 #endif
 
 /*
-- 
2.0.4



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

[PATCH v4 15/16] libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

The guest kernel will get the event channel interrupt information via
domain param HVM_PARAM_CALLBACK_IRQ. Initialize it here.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index 11a6f6e..d436167 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -900,8 +900,21 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
                                            struct xc_dom_image *dom)
 {
     int rc;
+    uint64_t val;
 
     assert(info->type == LIBXL_DOMAIN_TYPE_PV);
+
+    /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
+    val = (uint64_t)HVM_PARAM_CALLBACK_TYPE_PPI << HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
+    /* Active-low level-sensitive  */
+    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
+            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
+    val |= GUEST_EVTCHN_PPI & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
+    rc = xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CALLBACK_IRQ,
+                          val);
+    if (rc)
+        return rc;
+
     rc = libxl__prepare_dtb(gc, info, state, dom);
     if (rc) goto out;
 
-- 
2.0.4



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

[PATCH v4 16/16] libxl/arm: Add the size of ACPI tables to maxmem

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

While it defines the maximum size of guest ACPI tables in guest
memory layout, here it adds the size to set the target maxmem
to avoid providing less available memory for guest.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 tools/libxl/libxl_arm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index d436167..75b2589 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -103,6 +103,17 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
 int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
                               uint32_t domid)
 {
+    libxl_domain_build_info *const info = &d_config->b_info;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+
+    /* Add the size of ACPI tables to maxmem if ACPI is enabled for guest. */
+    if (libxl_defbool_val(info->acpi) &&
+        xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
+        LIBXL_MAXMEM_CONSTANT + GUEST_ACPI_SIZE / 1024) < 0) {
+        LOGE(ERROR, "Couldn't set max memory");
+        return ERROR_FAIL;
+    }
+
     return 0;
 }
 
-- 
2.0.4



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

Re: [PATCH v4 01/16] tools/libxl: Add an unified configuration option for ACPI

[Julien Grall]

Hi Shannon,

On 16/08/16 11:24, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Since the existing configuration option "u.hvm.acpi" is x86 specific and
> we want to reuse it on ARM as well, add a unified option "acpi" for
> x86 and ARM, and for ARM it's disabled by default.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_create.c  | 9 ++++++++-
>  tools/libxl/libxl_dm.c      | 6 ++++--
>  tools/libxl/libxl_types.idl | 4 ++++
>  tools/libxl/xl_cmdimpl.c    | 2 +-
>  4 files changed, 17 insertions(+), 4 deletions(-)

I think you need to update docs/man/xl.cfg.pod.5.in to mention the 
difference between x86 and ARM.

>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 08822e3..3043b1f 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -215,6 +215,12 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>      if (!b_info->event_channels)
>          b_info->event_channels = 1023;
>
> +#if defined(__arm__) || defined(__aarch64__)
> +    libxl_defbool_setdefault(&b_info->acpi, false);
> +#else
> +    libxl_defbool_setdefault(&b_info->acpi, true);
> +#endif
> +
>      switch (b_info->type) {
>      case LIBXL_DOMAIN_TYPE_HVM:
>          if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> @@ -454,7 +460,8 @@ int libxl__domain_build(libxl__gc *gc,
>          localents = libxl__calloc(gc, 9, sizeof(char *));
>          i = 0;
>          localents[i++] = "platform/acpi";
> -        localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
> +        localents[i++] = (libxl_defbool_val(info->acpi) &&
> +                         libxl_defbool_val(info->u.hvm.acpi)) ? "1" : "0";

It feels a bit weird to handle the backward compatibility directly in 
the code. It means that you have to do it everywhere hvm.acpi is used.

Can't you handle the backward compatibility directly in 
libxl__domain_build_info_setdefault?

>          localents[i++] = "platform/acpi_s3";
>          localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
>          localents[i++] = "platform/acpi_s4";
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index de16a59..12e4084 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -583,7 +583,8 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
>          if (b_info->u.hvm.soundhw) {
>              flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL);
>          }
> -        if (libxl_defbool_val(b_info->u.hvm.acpi)) {
> +        if (libxl_defbool_val(b_info->acpi) &&
> +            libxl_defbool_val(b_info->u.hvm.acpi)) {
>              flexarray_append(dm_args, "-acpi");
>          }
>          if (b_info->max_vcpus > 1) {
> @@ -1204,7 +1205,8 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
>          if (b_info->u.hvm.soundhw) {
>              flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL);
>          }
> -        if (!libxl_defbool_val(b_info->u.hvm.acpi)) {
> +        if (!(libxl_defbool_val(b_info->acpi) &&
> +             libxl_defbool_val(b_info->u.hvm.acpi))) {
>              flexarray_append(dm_args, "-no-acpi");
>          }
>          if (b_info->max_vcpus > 1) {
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 98bfc3a..a02446f 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -494,11 +494,15 @@ libxl_domain_build_info = Struct("domain_build_info",[
>      # Note that the partial device tree should avoid to use the phandle
>      # 65000 which is reserved by the toolstack.
>      ("device_tree",      string),
> +    ("acpi",             libxl_defbool),
>      ("u", KeyedUnion(None, libxl_domain_type, "type",
>                  [("hvm", Struct(None, [("firmware",         string),
>                                         ("bios",             libxl_bios_type),
>                                         ("pae",              libxl_defbool),
>                                         ("apic",             libxl_defbool),
> +                                       # The following acpi field is deprecated.
> +                                       # Please use the unified acpi field above
> +                                       # which works for both x86 and ARM.
>                                         ("acpi",             libxl_defbool),
>                                         ("acpi_s3",          libxl_defbool),
>                                         ("acpi_s4",          libxl_defbool),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 1d06598..be17702 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1547,6 +1547,7 @@ static void parse_config_data(const char *config_source,
>      b_info->cmdline = parse_cmdline(config);
>
>      xlu_cfg_get_defbool(config, "driver_domain", &c_info->driver_domain, 0);
> +    xlu_cfg_get_defbool(config, "acpi", &b_info->acpi, 0);
>
>      switch(b_info->type) {
>      case LIBXL_DOMAIN_TYPE_HVM:
> @@ -1576,7 +1577,6 @@ static void parse_config_data(const char *config_source,
>
>          xlu_cfg_get_defbool(config, "pae", &b_info->u.hvm.pae, 0);
>          xlu_cfg_get_defbool(config, "apic", &b_info->u.hvm.apic, 0);
> -        xlu_cfg_get_defbool(config, "acpi", &b_info->u.hvm.acpi, 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, "nx", &b_info->u.hvm.nx, 0);
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 02/16] libxl/arm: prepare for constructing ACPI tables

[Julien Grall]

On 16/08/16 11:24, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> It only constructs the ACPI tables for 64-bit ARM DomU when user enables
> acpi because 32-bit DomU doesn't support ACPI.

It would be worth to mention that the code is only built for 64-bit 
toolstack.

>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/Makefile          |  4 +++
>  tools/libxl/libxl_arm.c       | 23 +++++++++++++++-
>  tools/libxl/libxl_arm.h       | 44 +++++++++++++++++++++++++++++++
>  tools/libxl/libxl_arm_acpi.c  | 61 +++++++++++++++++++++++++++++++++++++++++++
>  xen/include/public/arch-arm.h |  4 +++
>  5 files changed, 135 insertions(+), 1 deletion(-)
>  create mode 100644 tools/libxl/libxl_arm.h
>  create mode 100644 tools/libxl/libxl_arm_acpi.c
>
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index a148374..6139bed 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -90,6 +90,10 @@ acpi:
>
>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o libxl_x86_acpi.o
>  LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
> +LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o

As mentioned above this will only enable ACPI when the toolstack is 
built for 64-bit. Although it might be possible to have 32-bit toolstack.

I guess it is fine for now, however I would like to avoid the #if 
defined(__aarch64__) in the code by introduce a libxl_arm_no_acpi.c 
similar to how it is done for other options (see in the Makefile).

> +
> +libxl_arm_acpi.o: libxl_arm_acpi.c
> +	$(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
>
>  ifeq ($(CONFIG_NetBSD),y)
>  LIBXL_OBJS-y += libxl_netbsd.o
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index bd3d611..8c7fc09 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -1,6 +1,7 @@
>  #include "libxl_internal.h"
>  #include "libxl_arch.h"
>  #include "libxl_libfdt_compat.h"
> +#include "libxl_arm.h"
>
>  #include <xc_dom.h>
>  #include <stdbool.h>
> @@ -885,8 +886,28 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>                                             libxl__domain_build_state *state,
>                                             struct xc_dom_image *dom)
>  {
> +    int rc;
> +
>      assert(info->type == LIBXL_DOMAIN_TYPE_PV);
> -    return libxl__prepare_dtb(gc, info, state, dom);
> +    rc = libxl__prepare_dtb(gc, info, state, dom);
> +    if (rc) goto out;
> +
> +    if (!libxl_defbool_val(info->acpi)) {
> +        LOG(DEBUG, "Generating ACPI tables is disabled by user.");
> +        rc = 0;
> +        goto out;
> +    }
> +
> +    if (strcmp(dom->guest_type, "xen-3.0-aarch64")) {

Can you add a comment in the code explaining that ACPI is only supported 
for 64-bit guest for now.

> +        LOG(ERROR, "Can not enable libxl option 'acpi' for %s", dom->guest_type);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    rc = libxl__prepare_acpi(gc, info, state, dom);
> +
> +out:
> +    return rc;
>  }
>
>  static void finalise_one_memory_node(libxl__gc *gc, void *fdt,
> diff --git a/tools/libxl/libxl_arm.h b/tools/libxl/libxl_arm.h
> new file mode 100644
> index 0000000..fe1c05f
> --- /dev/null
> +++ b/tools/libxl/libxl_arm.h
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright (C) 2016      Linaro Ltd.
> + *
> + * Author: Shannon Zhao <shannon.zhao@linaro.org>
> + *
> + * 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.
> + */
> +
> +#include "libxl_internal.h"
> +#include "libxl_arch.h"
> +
> +#include <xc_dom.h>
> +
> +#if defined(__aarch64__)
> +_hidden
> +int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
> +                        libxl__domain_build_state *state,
> +                        struct xc_dom_image *dom);
> +#else
> +_hidden
> +static inline int libxl__prepare_acpi(libxl__gc *gc,
> +                                      libxl_domain_build_info *info,
Can y
> +                                      libxl__domain_build_state *state,
> +                                      struct xc_dom_image *dom)
> +{
> +    return 0;

this should be either an ASSERT or return an error value to avoid any 
mis-usage.

> +}
> +#endif
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> new file mode 100644
> index 0000000..ec6cf08
> --- /dev/null
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -0,0 +1,61 @@
> +/*
> + * ARM DomU ACPI generation
> + *
> + * Copyright (C) 2016      Linaro Ltd.
> + *
> + * Author: Shannon Zhao <shannon.zhao@linaro.org>
> + *
> + * 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.
> + */
> +
> +#include "libxl_arm.h"
> +
> +#include <stdint.h>
> +

It might to add a comment explaining why you add those typedef because 
they are only use for acpi/*.h.

> +typedef uint8_t u8;
> +typedef uint16_t u16;
> +typedef uint32_t u32;
> +typedef uint64_t u64;
> +
> +#include <acpi/acconfig.h>
> +#include <acpi/actbl.h>
> +
> +int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
> +                        libxl__domain_build_state *state,
> +                        struct xc_dom_image *dom)
> +{
> +    const libxl_version_info *vers;
> +    int rc = 0;
> +
> +    vers = libxl_get_version_info(CTX);
> +    if (vers == NULL) {
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    LOG(DEBUG, "constructing ACPI tables for Xen version %d.%d guest",
> +        vers->xen_version_major, vers->xen_version_minor);
> +
> +    dom->acpi_modules[0].data = NULL;
> +    dom->acpi_modules[0].length = 0;
> +    dom->acpi_modules[0].guest_addr_out = GUEST_ACPI_BASE;
> +
> +out:
> +    return rc;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 870bc3b..0afd654 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -406,6 +406,10 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_GICV3_GICR0_BASE     0x03020000ULL    /* vCPU0 - vCPU127 */
>  #define GUEST_GICV3_GICR0_SIZE     0x01000000ULL
>
> +/* ACPI tables physical address */
> +#define GUEST_ACPI_BASE 0x20000000ULL
> +#define GUEST_ACPI_SIZE 0x00200000ULL
> +
>  /*
>   * 16MB == 4096 pages reserved for guest to use as a region to map its
>   * grant table in.
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 01/16] tools/libxl: Add an unified configuration option for ACPI

[Wei Liu]

On Tue, Aug 16, 2016 at 06:24:58PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Since the existing configuration option "u.hvm.acpi" is x86 specific and
> we want to reuse it on ARM as well, add a unified option "acpi" for
> x86 and ARM, and for ARM it's disabled by default.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_create.c  | 9 ++++++++-
>  tools/libxl/libxl_dm.c      | 6 ++++--
>  tools/libxl/libxl_types.idl | 4 ++++
>  tools/libxl/xl_cmdimpl.c    | 2 +-
>  4 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 08822e3..3043b1f 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -215,6 +215,12 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>      if (!b_info->event_channels)
>          b_info->event_channels = 1023;
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +    libxl_defbool_setdefault(&b_info->acpi, false);
> +#else
> +    libxl_defbool_setdefault(&b_info->acpi, true);
> +#endif
> +

I recently thought about how to handle the divergence between ARM and
x86. It would make sense to have an
libxl__arch_domain_build_info_acpi_setdefault here.

>      switch (b_info->type) {
>      case LIBXL_DOMAIN_TYPE_HVM:
>          if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> @@ -454,7 +460,8 @@ int libxl__domain_build(libxl__gc *gc,
>          localents = libxl__calloc(gc, 9, sizeof(char *));
>          i = 0;
>          localents[i++] = "platform/acpi";
> -        localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
> +        localents[i++] = (libxl_defbool_val(info->acpi) &&
> +                         libxl_defbool_val(info->u.hvm.acpi)) ? "1" : "0";

Please provide a function for this.

And the logic doesn't seem right. If the user sets u.hvm.acpi only,
(s)he should still have ACPI enabled.

Wei.

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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Wei Liu]

On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Construct ACPI RSDP table and add a helper to calculate the ACPI table
> checksum.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 6be9eb0..9432e44 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>  
> +#define ACPI_BUILD_APPNAME6 "XenARM"
> +#define ACPI_BUILD_APPNAME4 "Xen "
> +

Where do these come from? If they are from a spec, could you please add
a comment here?

>  enum {
>      RSDP,
>      XSDT,
> @@ -112,6 +115,37 @@ out:
>      return rc;
>  }
>  
> +static void calculate_checksum(void *table, uint32_t checksum_offset,
> +                               uint32_t length)
> +{
> +    uint8_t *p, sum = 0;
> +
> +    p = table;
> +    p[checksum_offset] = 0;
> +
> +    while ( length-- )

Coding style.

Wei.

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

Re: [PATCH v4 07/16] libxl/arm: Construct ACPI GTDT table

[Wei Liu]

On Tue, Aug 16, 2016 at 06:25:04PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Construct GTDT table with the interrupt information of timers.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 8cd1d9b..28fb6fe 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -24,10 +24,18 @@ typedef uint8_t u8;
>  typedef uint16_t u16;
>  typedef uint32_t u32;
>  typedef uint64_t u64;
> +typedef int64_t s64;
>  
>  #include <acpi/acconfig.h>
>  #include <acpi/actbl.h>
>  
> +#include <asm-generic/bitsperlong.h>

Hmm... This is likely to be Linux-centric. But I'm not sure if FreeBSD
or other BSDes have plan for Xen on ARM support.

CC Roger here.

> +#define ACPI_MACHINE_WIDTH __BITS_PER_LONG
> +#define COMPILER_DEPENDENT_INT64 int64_t
> +#define COMPILER_DEPENDENT_UINT64 uint64_t
> +
> +#include <acpi/actypes.h>
> +
>  _hidden
>  extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
> @@ -175,6 +183,26 @@ static void make_acpi_xsdt(libxl__gc *gc, struct xc_dom_image *dom,
>                         acpitables[XSDT].size);
>  }
>  
> +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom,
> +                           struct acpitable acpitables[])
> +{
> +    uint64_t offset = acpitables[GTDT].addr - GUEST_ACPI_BASE;
> +    struct acpi_table_gtdt *gtdt = (void *)dom->acpi_modules[0].data + offset;
> +
> +    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
> +    gtdt->non_secure_el1_flags =
> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
> +    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
> +    gtdt->virtual_timer_flags =
> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
> +
> +    make_acpi_header(&gtdt->header, "GTDT", acpitables[GTDT].size, 2);
> +    calculate_checksum(gtdt, offsetof(struct acpi_table_header, checksum),
> +                       acpitables[GTDT].size);
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
> @@ -205,6 +233,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>  
>      make_acpi_rsdp(gc, dom, acpitables);
>      make_acpi_xsdt(gc, dom, acpitables);
> +    make_acpi_gtdt(gc, dom, acpitables);
>  
>  out:
>      return rc;
> -- 
> 2.0.4
> 
> 

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

Re: [PATCH v4 00/16] Xen ARM DomU ACPI support

[Wei Liu]

On Tue, Aug 16, 2016 at 06:24:57PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> The design of this feature is described as below.
> Firstly, the toolstack (libxl) generates the ACPI tables according the
> number of vcpus and gic controller.
> 
> Then, it copies these ACPI tables to DomU non-RAM memory map space and
> passes them to UEFI firmware through the "ARM multiboot" protocol.
> 
> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
> and installs these tables like the usual way and passes both ACPI and DT
> information to the Xen DomU.
> 
> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
> since it's enough now.
> 
> This has been tested using guest kernel with the Dom0 ACPI support
> patches which could be fetched from linux master or:
> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
> 
> The UEFI binary could be fetched from or built from edk2 master branch:
> http://people.linaro.org/~shannon.zhao/DomU_ACPI/XEN_EFI.fd
> 
> This series can be fetched from:
> https://git.linaro.org/people/shannon.zhao/xen.git  domu_acpi_v4
> 

Thanks for posting this version and sorry for the late review.

I've skimmed the whole series and pointed out things I think should be
improved. I will leave reviewing all the table building code to ARM
maintainers.

Wei.

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

Re: [PATCH v4 14/16] public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI

[Wei Liu]

CC Jan and Andrew to review change in hvm/params.h

On Tue, Aug 16, 2016 at 06:25:11PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
> them in evtchn_fixup().
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  xen/arch/arm/domain_build.c     | 8 +++++---
>  xen/include/public/hvm/params.h | 4 ++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 60db9e4..94cd3ce 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
>             d->arch.evtchn_irq);
>  
>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
> -    val |= (2 << 8); /* Active-low level-sensitive  */
> -    val |= d->arch.evtchn_irq & 0xff;
> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
> +    /* Active-low level-sensitive  */
> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>  
>      /*
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index f7338a3..8a0327d 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -30,6 +30,7 @@
>   */
>  
>  #define HVM_PARAM_CALLBACK_IRQ 0
> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>  /*
>   * How should CPU0 event-channel notifications be delivered?
>   *
> @@ -66,6 +67,9 @@
>   * This is only used by ARM/ARM64 and masking/eoi the interrupt associated to
>   * the notification is handled by the interrupt controller.
>   */
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
>  #endif
>  
>  /*
> -- 
> 2.0.4
> 
> 

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

Re: [PATCH v4 14/16] public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI

[Jan Beulich]

>>> On 24.08.16 at 15:06, <wei.liu2@citrix.com> wrote:
> CC Jan and Andrew to review change in hvm/params.h

Well, it looks reasonable, but this really needs to first be approved by
the ARM maintainers.

Jan

> On Tue, Aug 16, 2016 at 06:25:11PM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>> 
>> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
>> them in evtchn_fixup().
>> 
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  xen/arch/arm/domain_build.c     | 8 +++++---
>>  xen/include/public/hvm/params.h | 4 ++++
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>> 
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 60db9e4..94cd3ce 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d, struct 
> kernel_info *kinfo)
>>             d->arch.evtchn_irq);
>>  
>>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
>> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
>> -    val |= (2 << 8); /* Active-low level-sensitive  */
>> -    val |= d->arch.evtchn_irq & 0xff;
>> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 
> HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
>> +    /* Active-low level-sensitive  */
>> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
>> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
>> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
>>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>>  
>>      /*
>> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
>> index f7338a3..8a0327d 100644
>> --- a/xen/include/public/hvm/params.h
>> +++ b/xen/include/public/hvm/params.h
>> @@ -30,6 +30,7 @@
>>   */
>>  
>>  #define HVM_PARAM_CALLBACK_IRQ 0
>> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>>  /*
>>   * How should CPU0 event-channel notifications be delivered?
>>   *
>> @@ -66,6 +67,9 @@
>>   * This is only used by ARM/ARM64 and masking/eoi the interrupt associated 
> to
>>   * the notification is handled by the interrupt controller.
>>   */
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
>>  #endif
>>  
>>  /*
>> -- 
>> 2.0.4
>> 
>> 




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

Re: [PATCH v4 07/16] libxl/arm: Construct ACPI GTDT table

[Roger Pau Monné]

On Wed, Aug 24, 2016 at 01:56:04PM +0100, Wei Liu wrote:
> On Tue, Aug 16, 2016 at 06:25:04PM +0800, Shannon Zhao wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > Construct GTDT table with the interrupt information of timers.
> > 
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> >  tools/libxl/libxl_arm_acpi.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> > index 8cd1d9b..28fb6fe 100644
> > --- a/tools/libxl/libxl_arm_acpi.c
> > +++ b/tools/libxl/libxl_arm_acpi.c
> > @@ -24,10 +24,18 @@ typedef uint8_t u8;
> >  typedef uint16_t u16;
> >  typedef uint32_t u32;
> >  typedef uint64_t u64;
> > +typedef int64_t s64;
> >  
> >  #include <acpi/acconfig.h>
> >  #include <acpi/actbl.h>
> >  
> > +#include <asm-generic/bitsperlong.h>
> 
> Hmm... This is likely to be Linux-centric. But I'm not sure if FreeBSD
> or other BSDes have plan for Xen on ARM support.

I would certainly love to see that happen, but I don't see myself working on 
that in the near future. In any case, this is a Linux-specific header, and 
should be included in libxl_osdeps.h. I would recommend that you use 
something like:

#ifndef BITS_PER_LONG
#ifdef _LP64
#define BITS_PER_LONG 64
#else
#define BITS_PER_LONG 32
#endif
#endif

So that it works on other systems also (or maybe you don't even need to 
include bitsperlong.h at all).

Roger.

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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Boris Ostrovsky]

On 08/16/2016 06:25 AM, Shannon Zhao wrote:
> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
> index d741ac5..7f50a33 100644
> --- a/tools/libacpi/Makefile
> +++ b/tools/libacpi/Makefile
> @@ -19,6 +19,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>  
>  # Sources to be generated
>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c  dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
> +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c
>  H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h)
>  
>  vpath iasl $(PATH)
> @@ -32,7 +33,7 @@ $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
>  	cd $(CURDIR)
>  
>  $(MK_DSDT): mk_dsdt.c
> -	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
> +	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -o $@ mk_dsdt.c
>  
>  $(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.asl: dsdt.asl dsdt_acpi_info.asl $(MK_DSDT)
>  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
> @@ -62,6 +63,18 @@ $(ACPI_BUILD_DIR)/dsdt_pvh.c: iasl $(ACPI_BUILD_DIR)/dsdt_pvh.asl
>  	echo "int dsdt_pvh_len=sizeof(dsdt_pvh);" >>$@
>  	rm -f $(ACPI_BUILD_DIR)/$*.aml $(ACPI_BUILD_DIR)/$*.hex
>  
> +$(ACPI_BUILD_DIR)/dsdt_anycpu_arm.asl: $(MK_DSDT)
> +	printf "DefinitionBlock (\"DSDT.aml\", \"DSDT\", 3, \"XenARM\", \"Xen DSDT\", 1)\n{" > $@
> +	$(MK_DSDT) --debug=$(debug) --arch arm >> $@
> +
> +$(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c: iasl $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.asl
> +	cd $(ACPI_BUILD_DIR)
> +	iasl -vs -p $* -tc $(ACPI_BUILD_DIR)/$*.asl
> +	sed -e 's/AmlCode/$*/g' $*.hex >$@
> +	echo "int $*_len=sizeof($*);" >>$@
> +	rm -f $*.aml $*.hex
> +	cd $(CURDIR)
> +

Note that x86 targets have been updated not to use 'cd'.


> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 6139bed..ce356d0 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -90,7 +90,10 @@ acpi:
>  
>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o libxl_x86_acpi.o
>  LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
> -LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o
> +LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o dsdt_anycpu_arm.o
> +
> +dsdt_anycpu_arm.c:
> +	$(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(shell pwd)

$(CURDIR) ?

(Both of these were requested by Jan when reviewing my x86 series so I
figured I'd mention them here as well)

-boris


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

Re: [PATCH v4 14/16] public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI

[Boris Ostrovsky]

On 08/16/2016 06:25 AM, Shannon Zhao wrote:
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index f7338a3..8a0327d 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -30,6 +30,7 @@
>   */
>  
>  #define HVM_PARAM_CALLBACK_IRQ 0
> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56

May be worth updating hvm_set_callback_via() with this parameter.

-boris


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

Re: [PATCH v4 01/16] tools/libxl: Add an unified configuration option for ACPI

[Shannon Zhao]

On 2016/8/24 20:50, Wei Liu wrote:
>> -        localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
>> > +        localents[i++] = (libxl_defbool_val(info->acpi) &&
>> > +                         libxl_defbool_val(info->u.hvm.acpi)) ? "1" : "0";
> Please provide a function for this.
> 
> And the logic doesn't seem right. If the user sets u.hvm.acpi only,
> (s)he should still have ACPI enabled.
info->acpi is true by defaut, so if user doesn't set info->acpi while
sets u.hvm.acpi only, both u.hvm.acpi and info->acpi will be true IIUC.

Thanks,
-- 
Shannon


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

Re: [PATCH v4 07/16] libxl/arm: Construct ACPI GTDT table

[Shannon Zhao]

On 2016/8/24 22:13, Roger Pau Monné wrote:
> On Wed, Aug 24, 2016 at 01:56:04PM +0100, Wei Liu wrote:
>> On Tue, Aug 16, 2016 at 06:25:04PM +0800, Shannon Zhao wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Construct GTDT table with the interrupt information of timers.
>>>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> ---
>>>  tools/libxl/libxl_arm_acpi.c | 29 +++++++++++++++++++++++++++++
>>>  1 file changed, 29 insertions(+)
>>>
>>> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>>> index 8cd1d9b..28fb6fe 100644
>>> --- a/tools/libxl/libxl_arm_acpi.c
>>> +++ b/tools/libxl/libxl_arm_acpi.c
>>> @@ -24,10 +24,18 @@ typedef uint8_t u8;
>>>  typedef uint16_t u16;
>>>  typedef uint32_t u32;
>>>  typedef uint64_t u64;
>>> +typedef int64_t s64;
>>>  
>>>  #include <acpi/acconfig.h>
>>>  #include <acpi/actbl.h>
>>>  
>>> +#include <asm-generic/bitsperlong.h>
>>
>> Hmm... This is likely to be Linux-centric. But I'm not sure if FreeBSD
>> or other BSDes have plan for Xen on ARM support.
> 
> I would certainly love to see that happen, but I don't see myself working on 
> that in the near future. In any case, this is a Linux-specific header, and 
> should be included in libxl_osdeps.h. I would recommend that you use 
> something like:
> 
> #ifndef BITS_PER_LONG
> #ifdef _LP64
> #define BITS_PER_LONG 64
> #else
> #define BITS_PER_LONG 32
> #endif
> #endif
> 
> So that it works on other systems also (or maybe you don't even need to 
> include bitsperlong.h at all).
> 
OK, will fix this.

Thanks,
-- 
Shannon


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

Re: [PATCH v4 00/16] Xen ARM DomU ACPI support

[Shannon Zhao]

On 2016/8/24 20:58, Wei Liu wrote:
> On Tue, Aug 16, 2016 at 06:24:57PM +0800, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > The design of this feature is described as below.
>> > Firstly, the toolstack (libxl) generates the ACPI tables according the
>> > number of vcpus and gic controller.
>> > 
>> > Then, it copies these ACPI tables to DomU non-RAM memory map space and
>> > passes them to UEFI firmware through the "ARM multiboot" protocol.
>> > 
>> > At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
>> > and installs these tables like the usual way and passes both ACPI and DT
>> > information to the Xen DomU.
>> > 
>> > Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
>> > since it's enough now.
>> > 
>> > This has been tested using guest kernel with the Dom0 ACPI support
>> > patches which could be fetched from linux master or:
>> > https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
>> > 
>> > The UEFI binary could be fetched from or built from edk2 master branch:
>> > http://people.linaro.org/~shannon.zhao/DomU_ACPI/XEN_EFI.fd
>> > 
>> > This series can be fetched from:
>> > https://git.linaro.org/people/shannon.zhao/xen.git  domu_acpi_v4
>> > 
> Thanks for posting this version and sorry for the late review.
> 
> I've skimmed the whole series and pointed out things I think should be
> improved. I will leave reviewing all the table building code to ARM
> maintainers.
Ok, thanks a lot for your help.

-- 
Shannon


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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Shannon Zhao]

On 2016/8/24 20:52, Wei Liu wrote:
> On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Construct ACPI RSDP table and add a helper to calculate the ACPI table
>> > checksum.
>> > 
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> >  tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 38 insertions(+)
>> > 
>> > diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>> > index 6be9eb0..9432e44 100644
>> > --- a/tools/libxl/libxl_arm_acpi.c
>> > +++ b/tools/libxl/libxl_arm_acpi.c
>> > @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>> >  _hidden
>> >  extern const int dsdt_anycpu_arm_len;
>> >  
>> > +#define ACPI_BUILD_APPNAME6 "XenARM"
>> > +#define ACPI_BUILD_APPNAME4 "Xen "
>> > +
> Where do these come from? If they are from a spec, could you please add
> a comment here?
> 
Not from some spec. Just fake a OEM for these tables like the
ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.

Thanks,
-- 
Shannon


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

Re: [PATCH v4 01/16] tools/libxl: Add an unified configuration option for ACPI

[Wei Liu]

On Thu, Aug 25, 2016 at 03:54:32PM +0800, Shannon Zhao wrote:
> 
> 
> On 2016/8/24 20:50, Wei Liu wrote:
> >> -        localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
> >> > +        localents[i++] = (libxl_defbool_val(info->acpi) &&
> >> > +                         libxl_defbool_val(info->u.hvm.acpi)) ? "1" : "0";
> > Please provide a function for this.
> > 
> > And the logic doesn't seem right. If the user sets u.hvm.acpi only,
> > (s)he should still have ACPI enabled.
> info->acpi is true by defaut, so if user doesn't set info->acpi while
> sets u.hvm.acpi only, both u.hvm.acpi and info->acpi will be true IIUC.

OK, that sounds sensible. But please could you document this expectation
in the to-be-created helper function? Thanks.

Wei.

> 
> Thanks,
> -- 
> Shannon
> 

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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Wei Liu]

On Thu, Aug 25, 2016 at 04:05:46PM +0800, Shannon Zhao wrote:
> 
> 
> On 2016/8/24 20:52, Wei Liu wrote:
> > On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
> >> > From: Shannon Zhao <shannon.zhao@linaro.org>
> >> > 
> >> > Construct ACPI RSDP table and add a helper to calculate the ACPI table
> >> > checksum.
> >> > 
> >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> > ---
> >> >  tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
> >> >  1 file changed, 38 insertions(+)
> >> > 
> >> > diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> >> > index 6be9eb0..9432e44 100644
> >> > --- a/tools/libxl/libxl_arm_acpi.c
> >> > +++ b/tools/libxl/libxl_arm_acpi.c
> >> > @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
> >> >  _hidden
> >> >  extern const int dsdt_anycpu_arm_len;
> >> >  
> >> > +#define ACPI_BUILD_APPNAME6 "XenARM"
> >> > +#define ACPI_BUILD_APPNAME4 "Xen "
> >> > +
> > Where do these come from? If they are from a spec, could you please add
> > a comment here?
> > 
> Not from some spec. Just fake a OEM for these tables like the
> ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.
> 

OK.  I'm no expert of ACPI, so if that's how things are done, I am fine
with that.

Wei.

> Thanks,
> -- 
> Shannon
> 

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

Re: [PATCH v4 01/16] tools/libxl: Add an unified configuration option for ACPI

[Shannon Zhao]

On 2016/8/25 17:05, Wei Liu wrote:
> On Thu, Aug 25, 2016 at 03:54:32PM +0800, Shannon Zhao wrote:
>> > 
>> > 
>> > On 2016/8/24 20:50, Wei Liu wrote:
>>>> > >> -        localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
>>>>> > >> > +        localents[i++] = (libxl_defbool_val(info->acpi) &&
>>>>> > >> > +                         libxl_defbool_val(info->u.hvm.acpi)) ? "1" : "0";
>>> > > Please provide a function for this.
>>> > > 
>>> > > And the logic doesn't seem right. If the user sets u.hvm.acpi only,
>>> > > (s)he should still have ACPI enabled.
>> > info->acpi is true by defaut, so if user doesn't set info->acpi while
>> > sets u.hvm.acpi only, both u.hvm.acpi and info->acpi will be true IIUC.
> OK, that sounds sensible. But please could you document this expectation
> in the to-be-created helper function? Thanks
Ok, will do.

Thanks,
-- 
Shannon


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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> It uses static DSDT table like the way x86 uses. Currently the DSDT
> table only contains processor device objects and it generates the
> maximal objects which so far is 128.
>
> Also only check iasl for aarch64 in configure since ACPI on ARM32 is not
> supported.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/configure               |  2 +-

The file tools/configure should not be modified manually. Instead you 
have to modify tools/configure.ac.

You can regenerate tools/configure, you can call ./autegen.sh. However, 
I would recommend you to not include the changes of configure and ask 
the committer to regenerate. This is because we use always use the same 
version of autotools to do generation in order to avoid spurious change.

>  tools/libacpi/Makefile        | 15 ++++++++++++-
>  tools/libacpi/mk_dsdt.c       | 51 ++++++++++++++++++++++++++++++++-----------
>  tools/libxl/Makefile          |  5 ++++-
>  tools/libxl/libxl_arm_acpi.c  |  5 +++++
>  xen/include/public/arch-arm.h |  3 +++
>  6 files changed, 65 insertions(+), 16 deletions(-)
>
> diff --git a/tools/configure b/tools/configure
> index 5b5dcce..48239c0 100755
> --- a/tools/configure
> +++ b/tools/configure
> @@ -7458,7 +7458,7 @@ then
>      as_fn_error $? "Unable to find xgettext, please install xgettext" "$LINENO" 5
>  fi
>  case "$host_cpu" in
> -i[3456]86|x86_64)
> +i[3456]86|x86_64|aarch64)
>      # Extract the first word of "iasl", so it can be a program name with args.
>  set dummy iasl; ac_word=$2
>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
> index d741ac5..7f50a33 100644
> --- a/tools/libacpi/Makefile
> +++ b/tools/libacpi/Makefile
> @@ -19,6 +19,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>
>  # Sources to be generated
>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c  dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
> +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c

Do we really want to generate dsdt_anycpu_arm.c even for x86? Similarly, 
do we want to generate x86 dsdt for ARM?

>  H_SRC = $(addprefix $(ACPI_BUILD_DIR)/, ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h)
>
>  vpath iasl $(PATH)
> @@ -32,7 +33,7 @@ $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
>  	cd $(CURDIR)
>
>  $(MK_DSDT): mk_dsdt.c
> -	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
> +	$(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -o $@ mk_dsdt.c

It would be useful to mention either in the code or in the commit 
message why you added __XEN_TOOLS__ here.

>
>  $(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.asl: dsdt.asl dsdt_acpi_info.asl $(MK_DSDT)
>  	awk 'NR > 1 {print s} {s=$$0}' $< > $@
> @@ -62,6 +63,18 @@ $(ACPI_BUILD_DIR)/dsdt_pvh.c: iasl $(ACPI_BUILD_DIR)/dsdt_pvh.asl
>  	echo "int dsdt_pvh_len=sizeof(dsdt_pvh);" >>$@
>  	rm -f $(ACPI_BUILD_DIR)/$*.aml $(ACPI_BUILD_DIR)/$*.hex
>
> +$(ACPI_BUILD_DIR)/dsdt_anycpu_arm.asl: $(MK_DSDT)
> +	printf "DefinitionBlock (\"DSDT.aml\", \"DSDT\", 3, \"XenARM\", \"Xen DSDT\", 1)\n{" > $@
> +	$(MK_DSDT) --debug=$(debug) --arch arm >> $@
> +
> +$(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c: iasl $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.asl
> +	cd $(ACPI_BUILD_DIR)
> +	iasl -vs -p $* -tc $(ACPI_BUILD_DIR)/$*.asl
> +	sed -e 's/AmlCode/$*/g' $*.hex >$@
> +	echo "int $*_len=sizeof($*);" >>$@
> +	rm -f $*.aml $*.hex
> +	cd $(CURDIR)
> +
>  iasl:
>  	@echo
>  	@echo "ACPI ASL compiler (iasl) is needed"
> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
> index 7d76784..f3ab28f 100644
> --- a/tools/libacpi/mk_dsdt.c
> +++ b/tools/libacpi/mk_dsdt.c
> @@ -18,6 +18,7 @@
>  #include <stdlib.h>
>  #include <stdbool.h>
>  #include <xen/hvm/hvm_info_table.h>
> +#include <xen/arch-arm.h>

arch-arm.h defines a lot of ARM specific constant. This is a call to 
misused them when built for x86.

Similarly, xen/hvm/hvm_info_table.h should not be included for ARM.

>
>  static unsigned int indent_level;
>  static bool debug = false;
> @@ -99,6 +100,7 @@ static struct option options[] = {
>      { "dm-version", 1, 0, 'q' },
>      { "debug", 1, 0, 'd' },
>      { "no-dm", 0, 0, 'n' },
> +    { "arch", 1, 0, 'a' },
>      { 0, 0, 0, 0 }
>  };
>
> @@ -106,7 +108,7 @@ int main(int argc, char **argv)
>  {
>      unsigned int slot, dev, intx, link, cpu, max_cpus = HVM_MAX_VCPUS;

Here an example why we should avoid to include x86 header for ARM. 
HVM_MAX_VCPUS is x86 specific.

>      dm_version dm_version = QEMU_XEN_TRADITIONAL;
> -    bool no_dm = 0;
> +    bool no_dm = 0, arch_is_arm = false;
>
>      for ( ; ; )
>      {
> @@ -145,6 +147,10 @@ int main(int argc, char **argv)
>          case 'n':
>              no_dm = 1;
>              break;
> +        case 'a':
> +            if (strcmp(optarg, "arm") == 0)
> +                arch_is_arm = true;
> +            break;
>          case 'd':
>              if (*optarg == 'y')
>                  debug = true;
> @@ -154,6 +160,9 @@ int main(int argc, char **argv)
>          }
>      }
>
> +    if (arch_is_arm)
> +        max_cpus = GUEST_MAX_VCPUS;
> +

And here GUEST_MAX_VCPUS is arm specific. I actually don't much 
understand why you added an option to mk_dsdt in order to generate ARM 
table. We will never build ARM table on x86, and vice versa.

>      /**** DSDT DefinitionBlock start ****/
>      /* (we append to existing DSDT definition block) */
>      indent_level++;
> @@ -161,19 +170,21 @@ int main(int argc, char **argv)
>      /**** Processor start ****/
>      push_block("Scope", "\\_SB");
>
> -    /* MADT checksum */
> -    stmt("OperationRegion", "MSUM, SystemMemory, \\_SB.MSUA, 1");
> -    push_block("Field", "MSUM, ByteAcc, NoLock, Preserve");
> -    indent(); printf("MSU, 8\n");
> -    pop_block();
> +    if (!arch_is_arm) {
> +        /* MADT checksum */
> +        stmt("OperationRegion", "MSUM, SystemMemory, \\_SB.MSUA, 1");
> +        push_block("Field", "MSUM, ByteAcc, NoLock, Preserve");
> +        indent(); printf("MSU, 8\n");
> +        pop_block();
>
> -    /* Processor object helpers. */
> -    push_block("Method", "PMAT, 2");
> -    push_block("If", "LLess(Arg0, NCPU)");
> -    stmt("Return", "ToBuffer(Arg1)");
> -    pop_block();
> -    stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
> -    pop_block();
> +        /* Processor object helpers. */
> +        push_block("Method", "PMAT, 2");
> +        push_block("If", "LLess(Arg0, NCPU)");
> +        stmt("Return", "ToBuffer(Arg1)");
> +        pop_block();
> +        stmt("Return", "Buffer() {0, 8, 0xff, 0xff, 0, 0, 0, 0}");
> +        pop_block();
> +    }
>
>      /* Define processor objects and control methods. */
>      for ( cpu = 0; cpu < max_cpus; cpu++)
> @@ -182,6 +193,12 @@ int main(int argc, char **argv)
>
>          stmt("Name", "_HID, \"ACPI0007\"");
>
> +        if (arch_is_arm) {
> +            stmt("Name", "_UID, %d", cpu);

_UID is not ARM specific. I am not sure why it is no added for x86.

> +            pop_block();
> +            continue;
> +        }
> +
>          /* Name this processor's MADT LAPIC descriptor. */
>          stmt("OperationRegion",
>               "MATR, SystemMemory, Add(\\_SB.MAPA, %d), 8", cpu*8);
> @@ -220,6 +237,14 @@ int main(int argc, char **argv)
>          pop_block();
>      }
>
> +    if (arch_is_arm) {
> +        pop_block();
> +        /**** Processor end ****/
> +        pop_block();
> +        /**** DSDT DefinitionBlock end ****/
> +        return 0;
> +    }
> +
>      /* Operation Region 'PRST': bitmask of online CPUs. */
>      stmt("OperationRegion", "PRST, SystemIO, 0xaf00, 32");
>      push_block("Field", "PRST, ByteAcc, NoLock, Preserve");
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 6139bed..ce356d0 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -90,7 +90,10 @@ acpi:
>
>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o libxl_x86_acpi.o
>  LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
> -LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o
> +LIBXL_OBJS-$(CONFIG_ARM_64) += libxl_arm_acpi.o dsdt_anycpu_arm.o
> +
> +dsdt_anycpu_arm.c:
> +	$(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(shell pwd)
>
>  libxl_arm_acpi.o: libxl_arm_acpi.c
>  	$(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index ec6cf08..087d028 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -28,6 +28,11 @@ typedef uint64_t u64;
>  #include <acpi/acconfig.h>
>  #include <acpi/actbl.h>
>
> +_hidden
> +extern const unsigned char dsdt_anycpu_arm[];
> +_hidden
> +extern const int dsdt_anycpu_arm_len;
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 0afd654..008a2a0 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -435,6 +435,9 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
>  #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
>
> +/* Current supported guest VCPUs */
> +#define GUEST_MAX_VCPUS 128

The number of vCPUS per guest supported depends whether Xen has been 
built for ARM32 or ARM64.

Also, because now we have two different place to define the number of 
vCPUS (here and include/asm-arm/config.h) it might be possible to have 
them differ by mistake.

I am not sure how to avoid the 2 definitions, so I would add a 
BUILD_BUG_ON in Xen to make sure that MAX_VIRT_CPUS is always <= to 
GUEST_MAX_VCPUS.

> +
>  /* Interrupts */
>  #define GUEST_TIMER_VIRT_PPI    27
>  #define GUEST_TIMER_PHYS_S_PPI  29
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Construct ACPI RSDP table and add a helper to calculate the ACPI table
> checksum.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 6be9eb0..9432e44 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>
> +#define ACPI_BUILD_APPNAME6 "XenARM"
> +#define ACPI_BUILD_APPNAME4 "Xen "
> +
>  enum {
>      RSDP,
>      XSDT,
> @@ -112,6 +115,37 @@ out:
>      return rc;
>  }
>
> +static void calculate_checksum(void *table, uint32_t checksum_offset,
> +                               uint32_t length)
> +{
> +    uint8_t *p, sum = 0;
> +
> +    p = table;
> +    p[checksum_offset] = 0;
> +
> +    while ( length-- )
> +        sum = sum + *p++;
> +
> +    p = table;
> +    p[checksum_offset] = -sum;
> +}
> +
> +static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
> +                           struct acpitable acpitables[])
> +{
> +    uint64_t offset = acpitables[RSDP].addr - GUEST_ACPI_BASE;
> +    struct acpi_table_rsdp *rsdp = (void *)dom->acpi_modules[0].data + offset;

Why do you cast to (void *)?

> +
> +    memcpy(rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
> +    memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
> +    rsdp->length = acpitables[RSDP].size;
> +    rsdp->revision = 0x02;
> +    rsdp->xsdt_physical_address = acpitables[XSDT].addr;
> +    calculate_checksum(rsdp,
> +                       offsetof(struct acpi_table_rsdp, extended_checksum),
> +                       acpitables[RSDP].size);
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
> @@ -137,6 +171,10 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>      dom->acpi_modules[0].guest_addr_out = GUEST_ACPI_BASE;
>
>      rc = libxl__estimate_acpi_size(gc, info, dom, xc_config, acpitables);
> +    if (rc)
> +        goto out;
> +
> +    make_acpi_rsdp(gc, dom, acpitables);
>
>  out:
>      return rc;
>

-- 
Julien Grall

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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Julien Grall]

Hi Shannon,

On 25/08/2016 04:05, Shannon Zhao wrote:
>
>
> On 2016/8/24 20:52, Wei Liu wrote:
>> On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Construct ACPI RSDP table and add a helper to calculate the ACPI table
>>>> checksum.
>>>>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> ---
>>>>  tools/libxl/libxl_arm_acpi.c | 38 ++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 38 insertions(+)
>>>>
>>>> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>>>> index 6be9eb0..9432e44 100644
>>>> --- a/tools/libxl/libxl_arm_acpi.c
>>>> +++ b/tools/libxl/libxl_arm_acpi.c
>>>> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>>>>  _hidden
>>>>  extern const int dsdt_anycpu_arm_len;
>>>>
>>>> +#define ACPI_BUILD_APPNAME6 "XenARM"
>>>> +#define ACPI_BUILD_APPNAME4 "Xen "
>>>> +
>> Where do these come from? If they are from a spec, could you please add
>> a comment here?
>>
> Not from some spec. Just fake a OEM for these tables like the
> ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.

In this case, why don't we re-use the one from x86?

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 06/16] libxl/arm: Construct ACPI XSDT table

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 9432e44..8cd1d9b 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -146,6 +146,35 @@ static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
>                         acpitables[RSDP].size);
>  }
>
> +static void make_acpi_header(struct acpi_table_header *h, const char *sig,
> +                             int len, uint8_t rev)

len should be size_t.

> +{
> +    memcpy(h->signature, sig, 4);
> +    h->length = len;
> +    h->revision = rev;
> +    memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> +    memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
> +    memcpy(h->oem_table_id + 4, sig, 4);
> +    h->oem_revision = 1;
> +    memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
> +    h->asl_compiler_revision = 1;
> +    h->checksum = 0;
> +}
> +
> +static void make_acpi_xsdt(libxl__gc *gc, struct xc_dom_image *dom,
> +                           struct acpitable acpitables[])
> +{
> +    uint64_t offset = acpitables[XSDT].addr - GUEST_ACPI_BASE;
> +    struct acpi_table_xsdt *xsdt = (void *)dom->acpi_modules[0].data + offset;
> +
> +    xsdt->table_offset_entry[0] = acpitables[MADT].addr;
> +    xsdt->table_offset_entry[1] = acpitables[GTDT].addr;
> +    xsdt->table_offset_entry[2] = acpitables[FADT].addr;
> +    make_acpi_header(&xsdt->header, "XSDT", acpitables[XSDT].size, 1);
> +    calculate_checksum(xsdt, offsetof(struct acpi_table_header, checksum),
> +                       acpitables[XSDT].size);
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
> @@ -175,6 +204,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>          goto out;
>
>      make_acpi_rsdp(gc, dom, acpitables);
> +    make_acpi_xsdt(gc, dom, acpitables);
>
>  out:
>      return rc;
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 07/16] libxl/arm: Construct ACPI GTDT table

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Construct GTDT table with the interrupt information of timers.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 8cd1d9b..28fb6fe 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -24,10 +24,18 @@ typedef uint8_t u8;
>  typedef uint16_t u16;
>  typedef uint32_t u32;
>  typedef uint64_t u64;
> +typedef int64_t s64;
>
>  #include <acpi/acconfig.h>
>  #include <acpi/actbl.h>
>
> +#include <asm-generic/bitsperlong.h>
> +#define ACPI_MACHINE_WIDTH __BITS_PER_LONG
> +#define COMPILER_DEPENDENT_INT64 int64_t
> +#define COMPILER_DEPENDENT_UINT64 uint64_t
> +
> +#include <acpi/actypes.h>
> +
>  _hidden
>  extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
> @@ -175,6 +183,26 @@ static void make_acpi_xsdt(libxl__gc *gc, struct xc_dom_image *dom,
>                         acpitables[XSDT].size);
>  }
>
> +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom,
> +                           struct acpitable acpitables[])
> +{
> +    uint64_t offset = acpitables[GTDT].addr - GUEST_ACPI_BASE;
> +    struct acpi_table_gtdt *gtdt = (void *)dom->acpi_modules[0].data + offset;
> +
> +    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
> +    gtdt->non_secure_el1_flags =
> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
> +    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
> +    gtdt->virtual_timer_flags =
> +                             (ACPI_LEVEL_SENSITIVE << ACPI_GTDT_INTERRUPT_MODE)
> +                             |(ACPI_ACTIVE_LOW << ACPI_GTDT_INTERRUPT_POLARITY);
> +

I don't see any setting for the field counter_block_address. From the 
ACPI spec, the field should be 0xFFFFFFFFFFFFFFFF when the counter 
control block is not available.

> +    make_acpi_header(&gtdt->header, "GTDT", acpitables[GTDT].size, 2);
> +    calculate_checksum(gtdt, offsetof(struct acpi_table_header, checksum),
> +                       acpitables[GTDT].size);
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
> @@ -205,6 +233,7 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>
>      make_acpi_rsdp(gc, dom, acpitables);
>      make_acpi_xsdt(gc, dom, acpitables);
> +    make_acpi_gtdt(gc, dom, acpitables);
>
>  out:
>      return rc;
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 08/16] libxl/arm: Factor MPIDR computing codes out as a helper

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Factor MPIDR computing codes out as a helper, so it could be shared
> between DT and ACPI.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Acked-by: Julien Grall <julien.grall@arm.com>

Regards,

> ---
>  tools/libxl/libxl_arm.c |  8 +-------
>  tools/libxl/libxl_arm.h | 11 +++++++++++
>  2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 8c7fc09..fa0497c 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -309,13 +309,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int nr_cpus,
>      for (i = 0; i < nr_cpus; i++) {
>          const char *name;
>
> -        /*
> -         * According to ARM CPUs bindings, the reg field should match
> -         * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
> -         * constructing the reg value of the guest at the moment, for it
> -         * is enough for the current max vcpu number.
> -         */
> -        mpidr_aff = (i & 0x0f) | (((i >> 4) & 0xff) << 8);
> +        mpidr_aff = libxl__compute_mpdir(i);
>          name = GCSPRINTF("cpu@%"PRIx64, mpidr_aff);
>
>          res = fdt_begin_node(fdt, name);
> diff --git a/tools/libxl/libxl_arm.h b/tools/libxl/libxl_arm.h
> index fe1c05f..5c8fbc6 100644
> --- a/tools/libxl/libxl_arm.h
> +++ b/tools/libxl/libxl_arm.h
> @@ -35,6 +35,17 @@ static inline int libxl__prepare_acpi(libxl__gc *gc,
>  }
>  #endif
>
> +static inline uint64_t libxl__compute_mpdir(unsigned int cpuid)
> +{
> +    /*
> +     * According to ARM CPUs bindings, the reg field should match
> +     * the MPIDR's affinity bits. We will use AFF0 and AFF1 when
> +     * constructing the reg value of the guest at the moment, for it
> +     * is enough for the current max vcpu number.
> +     */
> +    return (cpuid & 0x0f) | (((cpuid >> 4) & 0xff) << 8);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
>

-- 
Julien Grall

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

Re: [PATCH v4 09/16] libxl/arm: Construct ACPI MADT table

[Julien Grall]

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> According to the GIC version, construct the MADT table.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 84 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 28fb6fe..75dfcc2 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -203,6 +203,89 @@ static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom,
>                         acpitables[GTDT].size);
>  }
>
> +static void make_acpi_madt_gicc(void *table, int nr_cpus, uint64_t gicc_base)
> +{
> +    uint32_t i;

Please don't mix the type. Either i should be int or nr_cpus uint32_t.

> +    struct acpi_madt_generic_interrupt *gicc = table;
> +
> +    for (i = 0; i < nr_cpus; i++) {
> +        gicc->header.type = ACPI_MADT_TYPE_GENERIC_INTERRUPT;
> +        gicc->header.length = sizeof(*gicc);
> +        gicc->base_address = gicc_base;
> +        gicc->cpu_interface_number = i;
> +        gicc->arm_mpidr = libxl__compute_mpdir(i);
> +        gicc->uid = i;
> +        gicc->flags = ACPI_MADT_ENABLED;
> +        gicc++;
> +    }
> +}

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 10/16] libxl/arm: Construct ACPI FADT table

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm_acpi.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 75dfcc2..cb1c9df 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -286,6 +286,25 @@ out:
>      return rc;
>  }
>
> +static void make_acpi_fadt(libxl__gc *gc, struct xc_dom_image *dom,
> +                           struct acpitable acpitables[])
> +{
> +    uint64_t offset = acpitables[FADT].addr - GUEST_ACPI_BASE;
> +    struct acpi_table_fadt *fadt = (void *)dom->acpi_modules[0].data + offset;
> +
> +    /* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */
> +    fadt->flags = ACPI_FADT_HW_REDUCED;
> +    fadt->arm_boot_flags = (ACPI_FADT_PSCI_COMPLIANT) | ACPI_FADT_PSCI_USE_HVC;

NIT: The () are not necessary for ACPI_FADT_PSCI_COMPLIANT

> +
> +    /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */
> +    fadt->minor_revision = 0x1;
> +    fadt->dsdt = acpitables[DSDT].addr;
> +
> +    make_acpi_header(&fadt->header, "FACP", acpitables[FADT].size, 5);
> +    calculate_checksum(fadt, offsetof(struct acpi_table_header, checksum),
> +                       acpitables[FADT].size);
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>                          libxl__domain_build_state *state,
>                          struct xc_dom_image *dom)
> @@ -318,6 +337,10 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>      make_acpi_xsdt(gc, dom, acpitables);
>      make_acpi_gtdt(gc, dom, acpitables);
>      rc = make_acpi_madt(gc, dom, info->max_vcpus, xc_config, acpitables);
> +    if (rc)
> +        goto out;
> +
> +    make_acpi_fadt(gc, dom, acpitables);
>
>  out:
>      return rc;
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 12/16] libxl/arm: Factor finalise_one_memory_node as a gerneric function

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Rename finalise_one_memory_node to finalise_one_node and pass the node
> name via function parameter.
>
> This is useful for adding ACPI module which will be added by a later
> patch.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Acked-by: Julien Grall <julien.grall@arm.com>

Regards,

> ---
>  tools/libxl/libxl_arm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index fa0497c..b95fdf5 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -904,11 +904,11 @@ out:
>      return rc;
>  }
>
> -static void finalise_one_memory_node(libxl__gc *gc, void *fdt,
> -                                     uint64_t base, uint64_t size)
> +static void finalise_one_node(libxl__gc *gc, void *fdt, const char *uname,
> +                              uint64_t base, uint64_t size)
>  {
>      int node, res;
> -    const char *name = GCSPRINTF("/memory@%"PRIx64, base);
> +    const char *name = GCSPRINTF("%s@%"PRIx64, uname, base);
>
>      node = fdt_path_offset(fdt, name);
>      assert(node > 0);
> @@ -971,7 +971,7 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
>      for (i = 0; i < GUEST_RAM_BANKS; i++) {
>          const uint64_t size = (uint64_t)dom->rambank_size[i] << XC_PAGE_SHIFT;
>
> -        finalise_one_memory_node(gc, fdt, bankbase[i], size);
> +        finalise_one_node(gc, fdt, "/memory", bankbase[i], size);
>      }
>
>      debug_dump_fdt(gc, fdt);
>

-- 
Julien Grall

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

Re: [PATCH v4 13/16] libxl/arm: Add ACPI module

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Add the ARM Multiboot module for ACPI, so UEFI or DomU can get the base
> address of ACPI tables from it.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Acked-by: Julien Grall <julien.grall@arm.com>

> ---
>  docs/misc/arm/device-tree/acpi.txt | 24 ++++++++++++++++++++++++
>  tools/libxl/libxl_arm.c            | 24 ++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
>  create mode 100644 docs/misc/arm/device-tree/acpi.txt
>
> diff --git a/docs/misc/arm/device-tree/acpi.txt b/docs/misc/arm/device-tree/acpi.txt
> new file mode 100644
> index 0000000..3e70157
> --- /dev/null
> +++ b/docs/misc/arm/device-tree/acpi.txt
> @@ -0,0 +1,24 @@
> +DomU ACPI module
> +================================
> +
> +Xen toolstack passes the domU ACPI tables via a reference in the /chosen node of
> +the device tree.
> +
> +Each node contains the following properties:
> +
> +- compatible
> +
> +	"xen,guest-acpi", "multiboot,module"
> +
> +- reg
> +
> +	Specifies the physical address and the length of the module.
> +	RSDP table is always located at the beginning of this region.
> +
> +Examples
> +========
> +
> +	module@0x20000000 {
> +		compatible = "xen,guest-acpi", "multiboot,module";
> +		reg = <0x20000000 0x1234>;
> +	};
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index b95fdf5..11a6f6e 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -285,6 +285,25 @@ static int make_chosen_node(libxl__gc *gc, void *fdt, bool ramdisk,
>          if (res) return res;
>      }
>
> +    if (libxl_defbool_val(info->acpi)) {
> +        const uint64_t acpi_base = GUEST_ACPI_BASE;
> +        const char *name = GCSPRINTF("module@%"PRIx64, acpi_base);
> +
> +        res = fdt_begin_node(fdt, name);
> +        if (res) return res;
> +
> +        res = fdt_property_compat(gc, fdt, 2, "xen,guest-acpi",
> +                                  "multiboot,module");
> +        if (res) return res;
> +
> +        res = fdt_property_regs(gc, fdt, ROOT_ADDRESS_CELLS, ROOT_SIZE_CELLS,
> +                                1, 0, 0);
> +        if (res) return res;
> +
> +        res = fdt_end_node(fdt);
> +        if (res) return res;
> +    }
> +
>      res = fdt_end_node(fdt);
>      if (res) return res;
>
> @@ -974,6 +993,11 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
>          finalise_one_node(gc, fdt, "/memory", bankbase[i], size);
>      }
>
> +    if (dom->acpi_modules[0].data) {
> +        finalise_one_node(gc, fdt, "/chosen/module", GUEST_ACPI_BASE,
> +                          dom->acpi_modules[0].length);
> +    }
> +
>      debug_dump_fdt(gc, fdt);
>
>      return 0;
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 14/16] public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
> them in evtchn_fixup().
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  xen/arch/arm/domain_build.c     | 8 +++++---
>  xen/include/public/hvm/params.h | 4 ++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 60db9e4..94cd3ce 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
>             d->arch.evtchn_irq);
>
>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
> -    val |= (2 << 8); /* Active-low level-sensitive  */
> -    val |= d->arch.evtchn_irq & 0xff;
> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
> +    /* Active-low level-sensitive  */
> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;

The mask is pointless and a call to make things much worse if evtchn_irq 
is not a PPI (which will never happen).

>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>
>      /*
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index f7338a3..8a0327d 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -30,6 +30,7 @@
>   */
>
>  #define HVM_PARAM_CALLBACK_IRQ 0
> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>  /*
>   * How should CPU0 event-channel notifications be delivered?
>   *
> @@ -66,6 +67,9 @@
>   * This is only used by ARM/ARM64 and masking/eoi the interrupt associated to
>   * the notification is handled by the interrupt controller.
>   */
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff

Please drop the PPI_MASK, it is not correctly defined (there is only 16 
PPI going from 16 - 32) and pointless.

>  #endif
>
>  /*
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 16/16] libxl/arm: Add the size of ACPI tables to maxmem

[Julien Grall]

Hi Shannon,

On 16/08/2016 06:25, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> While it defines the maximum size of guest ACPI tables in guest
> memory layout, here it adds the size to set the target maxmem
> to avoid providing less available memory for guest.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  tools/libxl/libxl_arm.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index d436167..75b2589 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -103,6 +103,17 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
>                                uint32_t domid)
>  {
> +    libxl_domain_build_info *const info = &d_config->b_info;
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +
> +    /* Add the size of ACPI tables to maxmem if ACPI is enabled for guest. */
> +    if (libxl_defbool_val(info->acpi) &&
> +        xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> +        LIBXL_MAXMEM_CONSTANT + GUEST_ACPI_SIZE / 1024) < 0) {

Why can't we use the estimate size here? It would be better than 
increasing by a constant again the max size (I doubt the ACPI tables 
will be 2MB every time).

Also, this looks like quite unsafe. If someone decides to change the 
default size, (s)he would have to replicate the new algo here.

Wei, Ian, do you have any suggestion to avoid duplication?

> +        LOGE(ERROR, "Couldn't set max memory");
> +        return ERROR_FAIL;
> +    }
> +
>      return 0;
>  }
>
>

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 00/16] Xen ARM DomU ACPI support

[Julien Grall]

Hi Wei,

On 24/08/2016 08:58, Wei Liu wrote:
> On Tue, Aug 16, 2016 at 06:24:57PM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> The design of this feature is described as below.
>> Firstly, the toolstack (libxl) generates the ACPI tables according the
>> number of vcpus and gic controller.
>>
>> Then, it copies these ACPI tables to DomU non-RAM memory map space and
>> passes them to UEFI firmware through the "ARM multiboot" protocol.
>>
>> At last, UEFI gets the ACPI tables through the "ARM multiboot" protocol
>> and installs these tables like the usual way and passes both ACPI and DT
>> information to the Xen DomU.
>>
>> Currently libxl only generates RSDP, XSDT, GTDT, MADT, FADT, DSDT tables
>> since it's enough now.
>>
>> This has been tested using guest kernel with the Dom0 ACPI support
>> patches which could be fetched from linux master or:
>> https://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=efi/arm-xen
>>
>> The UEFI binary could be fetched from or built from edk2 master branch:
>> http://people.linaro.org/~shannon.zhao/DomU_ACPI/XEN_EFI.fd
>>
>> This series can be fetched from:
>> https://git.linaro.org/people/shannon.zhao/xen.git  domu_acpi_v4
>>
>
> Thanks for posting this version and sorry for the late review.
>
> I've skimmed the whole series and pointed out things I think should be
> improved. I will leave reviewing all the table building code to ARM
> maintainers.

I looked at the tables, they look good to me. Although, I made few 
comments on various place.

Cheers,

-- 
Julien Grall

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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Shannon Zhao]

On 2016/8/30 2:05, Julien Grall wrote:
> Hi Shannon,
> 
> On 25/08/2016 04:05, Shannon Zhao wrote:
>>
>>
>> On 2016/8/24 20:52, Wei Liu wrote:
>>> On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>
>>>>> Construct ACPI RSDP table and add a helper to calculate the ACPI table
>>>>> checksum.
>>>>>
>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>> ---
>>>>>  tools/libxl/libxl_arm_acpi.c | 38
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 38 insertions(+)
>>>>>
>>>>> diff --git a/tools/libxl/libxl_arm_acpi.c
>>>>> b/tools/libxl/libxl_arm_acpi.c
>>>>> index 6be9eb0..9432e44 100644
>>>>> --- a/tools/libxl/libxl_arm_acpi.c
>>>>> +++ b/tools/libxl/libxl_arm_acpi.c
>>>>> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>>>>>  _hidden
>>>>>  extern const int dsdt_anycpu_arm_len;
>>>>>
>>>>> +#define ACPI_BUILD_APPNAME6 "XenARM"
>>>>> +#define ACPI_BUILD_APPNAME4 "Xen "
>>>>> +
>>> Where do these come from? If they are from a spec, could you please add
>>> a comment here?
>>>
>> Not from some spec. Just fake a OEM for these tables like the
>> ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.
> 
> In this case, why don't we re-use the one from x86?
> 
While the ACPI_OEM_TABLE_ID and ACPI_CREATOR_ID of x86 are HVM specific,
I don't think it's proper for ARM.

Thanks,
-- 
Shannon


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

Re: [PATCH v4 16/16] libxl/arm: Add the size of ACPI tables to maxmem

[Shannon Zhao]

On 2016/8/30 3:07, Julien Grall wrote:
> Hi Shannon,
> 
> On 16/08/2016 06:25, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> While it defines the maximum size of guest ACPI tables in guest
>> memory layout, here it adds the size to set the target maxmem
>> to avoid providing less available memory for guest.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  tools/libxl/libxl_arm.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index d436167..75b2589 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -103,6 +103,17 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
>>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config
>> *d_config,
>>                                uint32_t domid)
>>  {
>> +    libxl_domain_build_info *const info = &d_config->b_info;
>> +    libxl_ctx *ctx = libxl__gc_owner(gc);
>> +
>> +    /* Add the size of ACPI tables to maxmem if ACPI is enabled for
>> guest. */
>> +    if (libxl_defbool_val(info->acpi) &&
>> +        xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
>> +        LIBXL_MAXMEM_CONSTANT + GUEST_ACPI_SIZE / 1024) < 0) {
> 
> Why can't we use the estimate size here? It would be better than
> increasing by a constant again the max size (I doubt the ACPI tables
> will be 2MB every time).
> 
The estimate action happens after libxl__arch_domain_create(), I think.
So it can't get the estimate size here.

Also, it's better to add the max size so that it doesn't have to change
here when it adds other contents to the ACPI tables.

Thanks,
-- 
Shannon


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

Re: [PATCH v4 14/16] public/hvm/params.h: Add macros for HVM_PARAM_CALLBACK_TYPE_PPI

[Shannon Zhao]

On 2016/8/30 3:00, Julien Grall wrote:
> Hi Shannon,
> 
> On 16/08/2016 06:25, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Add macros for HVM_PARAM_CALLBACK_TYPE_PPI operation values and update
>> them in evtchn_fixup().
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  xen/arch/arm/domain_build.c     | 8 +++++---
>>  xen/include/public/hvm/params.h | 4 ++++
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 60db9e4..94cd3ce 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -2019,9 +2019,11 @@ static void evtchn_fixup(struct domain *d,
>> struct kernel_info *kinfo)
>>             d->arch.evtchn_irq);
>>
>>      /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
>> -    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
>> -    val |= (2 << 8); /* Active-low level-sensitive  */
>> -    val |= d->arch.evtchn_irq & 0xff;
>> +    val = (u64)HVM_PARAM_CALLBACK_TYPE_PPI <<
>> HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT;
>> +    /* Active-low level-sensitive  */
>> +    val |= (HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL <<
>> +            HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT);
>> +    val |= d->arch.evtchn_irq & HVM_PARAM_CALLBACK_TYPE_PPI_MASK;
> 
> The mask is pointless and a call to make things much worse if evtchn_irq
> is not a PPI (which will never happen).
> 
>>      d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
>>
>>      /*
>> diff --git a/xen/include/public/hvm/params.h
>> b/xen/include/public/hvm/params.h
>> index f7338a3..8a0327d 100644
>> --- a/xen/include/public/hvm/params.h
>> +++ b/xen/include/public/hvm/params.h
>> @@ -30,6 +30,7 @@
>>   */
>>
>>  #define HVM_PARAM_CALLBACK_IRQ 0
>> +#define HVM_PARAM_CALLBACK_IRQ_TYPE_SHIFT     56
>>  /*
>>   * How should CPU0 event-channel notifications be delivered?
>>   *
>> @@ -66,6 +67,9 @@
>>   * This is only used by ARM/ARM64 and masking/eoi the interrupt
>> associated to
>>   * the notification is handled by the interrupt controller.
>>   */
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_SHIFT     8
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_FLAG_LOW_LEVEL 2
>> +#define HVM_PARAM_CALLBACK_TYPE_PPI_MASK           0xff
> 
> Please drop the PPI_MASK, it is not correctly defined (there is only 16
> PPI going from 16 - 32) and pointless.
> 
Sure. Thanks.

-- 
Shannon


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

Re: [PATCH v4 07/16] libxl/arm: Construct ACPI GTDT table

[Shannon Zhao]

On 2016/8/30 2:16, Julien Grall wrote:
> On 16/08/2016 06:25, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Construct GTDT table with the interrupt information of timers.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  tools/libxl/libxl_arm_acpi.c | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>> index 8cd1d9b..28fb6fe 100644
>> --- a/tools/libxl/libxl_arm_acpi.c
>> +++ b/tools/libxl/libxl_arm_acpi.c
>> @@ -24,10 +24,18 @@ typedef uint8_t u8;
>>  typedef uint16_t u16;
>>  typedef uint32_t u32;
>>  typedef uint64_t u64;
>> +typedef int64_t s64;
>>
>>  #include <acpi/acconfig.h>
>>  #include <acpi/actbl.h>
>>
>> +#include <asm-generic/bitsperlong.h>
>> +#define ACPI_MACHINE_WIDTH __BITS_PER_LONG
>> +#define COMPILER_DEPENDENT_INT64 int64_t
>> +#define COMPILER_DEPENDENT_UINT64 uint64_t
>> +
>> +#include <acpi/actypes.h>
>> +
>>  _hidden
>>  extern const unsigned char dsdt_anycpu_arm[];
>>  _hidden
>> @@ -175,6 +183,26 @@ static void make_acpi_xsdt(libxl__gc *gc, struct
>> xc_dom_image *dom,
>>                         acpitables[XSDT].size);
>>  }
>>
>> +static void make_acpi_gtdt(libxl__gc *gc, struct xc_dom_image *dom,
>> +                           struct acpitable acpitables[])
>> +{
>> +    uint64_t offset = acpitables[GTDT].addr - GUEST_ACPI_BASE;
>> +    struct acpi_table_gtdt *gtdt = (void *)dom->acpi_modules[0].data
>> + offset;
>> +
>> +    gtdt->non_secure_el1_interrupt = GUEST_TIMER_PHYS_NS_PPI;
>> +    gtdt->non_secure_el1_flags =
>> +                             (ACPI_LEVEL_SENSITIVE <<
>> ACPI_GTDT_INTERRUPT_MODE)
>> +                             |(ACPI_ACTIVE_LOW <<
>> ACPI_GTDT_INTERRUPT_POLARITY);
>> +    gtdt->virtual_timer_interrupt = GUEST_TIMER_VIRT_PPI;
>> +    gtdt->virtual_timer_flags =
>> +                             (ACPI_LEVEL_SENSITIVE <<
>> ACPI_GTDT_INTERRUPT_MODE)
>> +                             |(ACPI_ACTIVE_LOW <<
>> ACPI_GTDT_INTERRUPT_POLARITY);
>> +
> 
> I don't see any setting for the field counter_block_address. From the
> ACPI spec, the field should be 0xFFFFFFFFFFFFFFFF when the counter
> control block is not available
Oops, will add. Thanks.

-- 
Shannon


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

Re: [PATCH v4 16/16] libxl/arm: Add the size of ACPI tables to maxmem

[Wei Liu]

On Mon, Aug 29, 2016 at 03:07:07PM -0400, Julien Grall wrote:
> Hi Shannon,
> 
> On 16/08/2016 06:25, Shannon Zhao wrote:
> >From: Shannon Zhao <shannon.zhao@linaro.org>
> >
> >While it defines the maximum size of guest ACPI tables in guest
> >memory layout, here it adds the size to set the target maxmem
> >to avoid providing less available memory for guest.
> >
> >Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >---
> > tools/libxl/libxl_arm.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> >diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> >index d436167..75b2589 100644
> >--- a/tools/libxl/libxl_arm.c
> >+++ b/tools/libxl/libxl_arm.c
> >@@ -103,6 +103,17 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
> > int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config,
> >                               uint32_t domid)
> > {
> >+    libxl_domain_build_info *const info = &d_config->b_info;
> >+    libxl_ctx *ctx = libxl__gc_owner(gc);
> >+
> >+    /* Add the size of ACPI tables to maxmem if ACPI is enabled for guest. */
> >+    if (libxl_defbool_val(info->acpi) &&
> >+        xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> >+        LIBXL_MAXMEM_CONSTANT + GUEST_ACPI_SIZE / 1024) < 0) {
> 
> Why can't we use the estimate size here? It would be better than increasing
> by a constant again the max size (I doubt the ACPI tables will be 2MB every
> time).
> 
> Also, this looks like quite unsafe. If someone decides to change the default
> size, (s)he would have to replicate the new algo here.
> 
> Wei, Ian, do you have any suggestion to avoid duplication?
> 

Provide a function to calculate the value needed?

> >+        LOGE(ERROR, "Couldn't set max memory");
> >+        return ERROR_FAIL;
> >+    }
> >+
> >     return 0;
> > }
> >
> >
> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [PATCH v4 16/16] libxl/arm: Add the size of ACPI tables to maxmem

[Shannon Zhao]

On 2016/8/30 3:07, Julien Grall wrote:
> Hi Shannon,
> 
> On 16/08/2016 06:25, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> While it defines the maximum size of guest ACPI tables in guest
>> memory layout, here it adds the size to set the target maxmem
>> to avoid providing less available memory for guest.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  tools/libxl/libxl_arm.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index d436167..75b2589 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -103,6 +103,17 @@ int libxl__arch_domain_save_config(libxl__gc *gc,
>>  int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config
>> *d_config,
>>                                uint32_t domid)
>>  {
>> +    libxl_domain_build_info *const info = &d_config->b_info;
>> +    libxl_ctx *ctx = libxl__gc_owner(gc);
>> +
>> +    /* Add the size of ACPI tables to maxmem if ACPI is enabled for
>> guest. */
>> +    if (libxl_defbool_val(info->acpi) &&
>> +        xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
>> +        LIBXL_MAXMEM_CONSTANT + GUEST_ACPI_SIZE / 1024) < 0) {
> 
> Why can't we use the estimate size here? It would be better than
> increasing by a constant again the max size (I doubt the ACPI tables
> will be 2MB every time).
> 
Rethink about this. I think it could call the
libxl__estimate_acpi_size() here.

Thanks,
-- 
Shannon


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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Julien Grall]

Hi Shannon,

On 30/08/16 02:21, Shannon Zhao wrote:
>
>
> On 2016/8/30 2:05, Julien Grall wrote:
>> Hi Shannon,
>>
>> On 25/08/2016 04:05, Shannon Zhao wrote:
>>>
>>>
>>> On 2016/8/24 20:52, Wei Liu wrote:
>>>> On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
>>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>>
>>>>>> Construct ACPI RSDP table and add a helper to calculate the ACPI table
>>>>>> checksum.
>>>>>>
>>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>> ---
>>>>>>  tools/libxl/libxl_arm_acpi.c | 38
>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 38 insertions(+)
>>>>>>
>>>>>> diff --git a/tools/libxl/libxl_arm_acpi.c
>>>>>> b/tools/libxl/libxl_arm_acpi.c
>>>>>> index 6be9eb0..9432e44 100644
>>>>>> --- a/tools/libxl/libxl_arm_acpi.c
>>>>>> +++ b/tools/libxl/libxl_arm_acpi.c
>>>>>> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>>>>>>  _hidden
>>>>>>  extern const int dsdt_anycpu_arm_len;
>>>>>>
>>>>>> +#define ACPI_BUILD_APPNAME6 "XenARM"
>>>>>> +#define ACPI_BUILD_APPNAME4 "Xen "
>>>>>> +
>>>> Where do these come from? If they are from a spec, could you please add
>>>> a comment here?
>>>>
>>> Not from some spec. Just fake a OEM for these tables like the
>>> ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.
>>
>> In this case, why don't we re-use the one from x86?
>>
> While the ACPI_OEM_TABLE_ID and ACPI_CREATOR_ID of x86 are HVM specific,
> I don't think it's proper for ARM.

IIRC we have an OEM ID and Creator ID reserved for Xen. Stefano can you 
confirm?

In any case, it would be better if we re-use the existing one. It does 
not hurt to use a different name. For instance the signature of the MADT 
table is "APIC" with is x86 specific...

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 05/16] libxl/arm: Construct ACPI RSDP table

[Stefano Stabellini]

On Tue, 30 Aug 2016, Julien Grall wrote:
> Hi Shannon,
> 
> On 30/08/16 02:21, Shannon Zhao wrote:
> > 
> > 
> > On 2016/8/30 2:05, Julien Grall wrote:
> > > Hi Shannon,
> > > 
> > > On 25/08/2016 04:05, Shannon Zhao wrote:
> > > > 
> > > > 
> > > > On 2016/8/24 20:52, Wei Liu wrote:
> > > > > On Tue, Aug 16, 2016 at 06:25:02PM +0800, Shannon Zhao wrote:
> > > > > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > > > 
> > > > > > > Construct ACPI RSDP table and add a helper to calculate the ACPI
> > > > > > > table
> > > > > > > checksum.
> > > > > > > 
> > > > > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > > > ---
> > > > > > >  tools/libxl/libxl_arm_acpi.c | 38
> > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 38 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/tools/libxl/libxl_arm_acpi.c
> > > > > > > b/tools/libxl/libxl_arm_acpi.c
> > > > > > > index 6be9eb0..9432e44 100644
> > > > > > > --- a/tools/libxl/libxl_arm_acpi.c
> > > > > > > +++ b/tools/libxl/libxl_arm_acpi.c
> > > > > > > @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
> > > > > > >  _hidden
> > > > > > >  extern const int dsdt_anycpu_arm_len;
> > > > > > > 
> > > > > > > +#define ACPI_BUILD_APPNAME6 "XenARM"
> > > > > > > +#define ACPI_BUILD_APPNAME4 "Xen "
> > > > > > > +
> > > > > Where do these come from? If they are from a spec, could you please
> > > > > add
> > > > > a comment here?
> > > > > 
> > > > Not from some spec. Just fake a OEM for these tables like the
> > > > ACPI_OEM_ID, ACPI_CREATOR_ID used by x86.
> > > 
> > > In this case, why don't we re-use the one from x86?
> > > 
> > While the ACPI_OEM_TABLE_ID and ACPI_CREATOR_ID of x86 are HVM specific,
> > I don't think it's proper for ARM.
> 
> IIRC we have an OEM ID and Creator ID reserved for Xen. Stefano can you
> confirm?
>
> In any case, it would be better if we re-use the existing one. It does not
> hurt to use a different name. For instance the signature of the MADT table is
> "APIC" with is x86 specific...

I think it makes sense to use the same OEM id as x86, which seems to be
"Xen", but I am not sure about who reserved it, it predates me. I would
change the creator id, because it is "HVML" on x86, for "HVM Loader".
Maybe we could use "xl". FYI creator id is the id of the utility that
generated the tables.

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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Shannon Zhao]

On 2016/8/30 1:46, Julien Grall wrote:
> Hi Shannon,
> 
> On 16/08/2016 06:25, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> It uses static DSDT table like the way x86 uses. Currently the DSDT
>> table only contains processor device objects and it generates the
>> maximal objects which so far is 128.
>>
>> Also only check iasl for aarch64 in configure since ACPI on ARM32 is not
>> supported.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  tools/configure               |  2 +-
> 
> The file tools/configure should not be modified manually. Instead you
> have to modify tools/configure.ac.
> 
> You can regenerate tools/configure, you can call ./autegen.sh. However,
> I would recommend you to not include the changes of configure and ask
> the committer to regenerate. This is because we use always use the same
> version of autotools to do generation in order to avoid spurious change.
> 
Ok, will fix.

>> diff --git a/xen/include/public/arch-arm.h
>> b/xen/include/public/arch-arm.h
>> index 0afd654..008a2a0 100644
>> --- a/xen/include/public/arch-arm.h
>> +++ b/xen/include/public/arch-arm.h
>> @@ -435,6 +435,9 @@ typedef uint64_t xen_callback_t;
>>  #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
>>  #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
>>
>> +/* Current supported guest VCPUs */
>> +#define GUEST_MAX_VCPUS 128
> 
> The number of vCPUS per guest supported depends whether Xen has been
> built for ARM32 or ARM64.
> 
> Also, because now we have two different place to define the number of
> vCPUS (here and include/asm-arm/config.h) it might be possible to have
> them differ by mistake.
> 
> I am not sure how to avoid the 2 definitions, so I would add a
> BUILD_BUG_ON in Xen to make sure that MAX_VIRT_CPUS is always <= to
> GUEST_MAX_VCPUS.
> 
It has the below check. So could we just define GUEST_MAX_VCPUS as
(GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE)?

BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) <
MAX_VIRT_CPUS);

Thanks,
-- 
Shannon


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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Julien Grall]

Hi Shannon,

On 31/08/16 07:37, Shannon Zhao wrote:
> On 2016/8/30 1:46, Julien Grall wrote:
>> On 16/08/2016 06:25, Shannon Zhao wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> It uses static DSDT table like the way x86 uses. Currently the DSDT
>>> table only contains processor device objects and it generates the
>>> maximal objects which so far is 128.
>>>
>>> Also only check iasl for aarch64 in configure since ACPI on ARM32 is not
>>> supported.
>>>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> ---
>>>  tools/configure               |  2 +-
>>
>> The file tools/configure should not be modified manually. Instead you
>> have to modify tools/configure.ac.
>>
>> You can regenerate tools/configure, you can call ./autegen.sh. However,
>> I would recommend you to not include the changes of configure and ask
>> the committer to regenerate. This is because we use always use the same
>> version of autotools to do generation in order to avoid spurious change.
>>
> Ok, will fix.
>
>>> diff --git a/xen/include/public/arch-arm.h
>>> b/xen/include/public/arch-arm.h
>>> index 0afd654..008a2a0 100644
>>> --- a/xen/include/public/arch-arm.h
>>> +++ b/xen/include/public/arch-arm.h
>>> @@ -435,6 +435,9 @@ typedef uint64_t xen_callback_t;
>>>  #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
>>>  #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
>>>
>>> +/* Current supported guest VCPUs */
>>> +#define GUEST_MAX_VCPUS 128
>>
>> The number of vCPUS per guest supported depends whether Xen has been
>> built for ARM32 or ARM64.
>>
>> Also, because now we have two different place to define the number of
>> vCPUS (here and include/asm-arm/config.h) it might be possible to have
>> them differ by mistake.
>>
>> I am not sure how to avoid the 2 definitions, so I would add a
>> BUILD_BUG_ON in Xen to make sure that MAX_VIRT_CPUS is always <= to
>> GUEST_MAX_VCPUS.
>>
> It has the below check. So could we just define GUEST_MAX_VCPUS as
> (GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE)?

I much prefer hardcoding the value. It will be easier to catch any issue 
if we decide to use multiple re-distributor regions.

Stefano, do you have any opinions?

>
> BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) <
> MAX_VIRT_CPUS);
>
> Thanks,

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Stefano Stabellini]

On Wed, 31 Aug 2016, Julien Grall wrote:
> Hi Shannon,
> 
> On 31/08/16 07:37, Shannon Zhao wrote:
> > On 2016/8/30 1:46, Julien Grall wrote:
> > > On 16/08/2016 06:25, Shannon Zhao wrote:
> > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > > 
> > > > It uses static DSDT table like the way x86 uses. Currently the DSDT
> > > > table only contains processor device objects and it generates the
> > > > maximal objects which so far is 128.
> > > > 
> > > > Also only check iasl for aarch64 in configure since ACPI on ARM32 is not
> > > > supported.
> > > > 
> > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > > ---
> > > >  tools/configure               |  2 +-
> > > 
> > > The file tools/configure should not be modified manually. Instead you
> > > have to modify tools/configure.ac.
> > > 
> > > You can regenerate tools/configure, you can call ./autegen.sh. However,
> > > I would recommend you to not include the changes of configure and ask
> > > the committer to regenerate. This is because we use always use the same
> > > version of autotools to do generation in order to avoid spurious change.
> > > 
> > Ok, will fix.
> > 
> > > > diff --git a/xen/include/public/arch-arm.h
> > > > b/xen/include/public/arch-arm.h
> > > > index 0afd654..008a2a0 100644
> > > > --- a/xen/include/public/arch-arm.h
> > > > +++ b/xen/include/public/arch-arm.h
> > > > @@ -435,6 +435,9 @@ typedef uint64_t xen_callback_t;
> > > >  #define GUEST_RAM_BANK_BASES   { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
> > > >  #define GUEST_RAM_BANK_SIZES   { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
> > > > 
> > > > +/* Current supported guest VCPUs */
> > > > +#define GUEST_MAX_VCPUS 128
> > > 
> > > The number of vCPUS per guest supported depends whether Xen has been
> > > built for ARM32 or ARM64.
> > > 
> > > Also, because now we have two different place to define the number of
> > > vCPUS (here and include/asm-arm/config.h) it might be possible to have
> > > them differ by mistake.
> > > 
> > > I am not sure how to avoid the 2 definitions, so I would add a
> > > BUILD_BUG_ON in Xen to make sure that MAX_VIRT_CPUS is always <= to
> > > GUEST_MAX_VCPUS.
> > > 
> > It has the below check. So could we just define GUEST_MAX_VCPUS as
> > (GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE)?
> 
> I much prefer hardcoding the value. It will be easier to catch any issue if we
> decide to use multiple re-distributor regions.
> 
> Stefano, do you have any opinions?

I agree with you. It is going to be much easier to catch any mistakes
with the hardcoded value.


> > 
> > BUILD_BUG_ON((GUEST_GICV3_GICR0_SIZE / GUEST_GICV3_RDIST_STRIDE) <
> > MAX_VIRT_CPUS);
> > 
> > Thanks,

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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Shannon Zhao]

On 2016/8/30 1:46, Julien Grall wrote:
>> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
>> index d741ac5..7f50a33 100644
>> --- a/tools/libacpi/Makefile
>> +++ b/tools/libacpi/Makefile
>> @@ -19,6 +19,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>>
>>  # Sources to be generated
>>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c 
>> dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
>> +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c
> 
> Do we really want to generate dsdt_anycpu_arm.c even for x86? Similarly,
> do we want to generate x86 dsdt for ARM
No need I think.
Boris, will you change this in your next version?

Thanks,
-- 
Shannon


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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Boris Ostrovsky]

On 08/31/2016 11:18 PM, Shannon Zhao wrote:
>
> On 2016/8/30 1:46, Julien Grall wrote:
>>> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
>>> index d741ac5..7f50a33 100644
>>> --- a/tools/libacpi/Makefile
>>> +++ b/tools/libacpi/Makefile
>>> @@ -19,6 +19,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>>>
>>>  # Sources to be generated
>>>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c 
>>> dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
>>> +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c
>> Do we really want to generate dsdt_anycpu_arm.c even for x86? Similarly,
>> do we want to generate x86 dsdt for ARM
> No need I think.
> Boris, will you change this in your next version?

You mean adding something along the lines of 'C_SRC-$(CONFIG_X86)'? Yes,
it's probably a good idea. I can add that.

-boris

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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Shannon Zhao]

On 2016/9/1 20:53, Boris Ostrovsky wrote:
> On 08/31/2016 11:18 PM, Shannon Zhao wrote:
>> >
>> > On 2016/8/30 1:46, Julien Grall wrote:
>>>> >>> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
>>>> >>> index d741ac5..7f50a33 100644
>>>> >>> --- a/tools/libacpi/Makefile
>>>> >>> +++ b/tools/libacpi/Makefile
>>>> >>> @@ -19,6 +19,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>>>> >>>
>>>> >>>  # Sources to be generated
>>>> >>>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c 
>>>> >>> dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
>>>> >>> +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c
>>> >> Do we really want to generate dsdt_anycpu_arm.c even for x86? Similarly,
>>> >> do we want to generate x86 dsdt for ARM
>> > No need I think.
>> > Boris, will you change this in your next version?
> You mean adding something along the lines of 'C_SRC-$(CONFIG_X86)'? Yes,
> it's probably a good idea. I can add that.
Yes. Then I'll rebase my series on your new version. BTW, when will you
send out your new series?

Thanks,
-- 
Shannon


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

Re: [PATCH v4 03/16] libxl/arm: Generate static ACPI DSDT table

[Boris Ostrovsky]

On 09/01/2016 08:55 PM, Shannon Zhao wrote:
>
> On 2016/9/1 20:53, Boris Ostrovsky wrote:
>> On 08/31/2016 11:18 PM, Shannon Zhao wrote:
>>>> On 2016/8/30 1:46, Julien Grall wrote:
>>>>>>>> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
>>>>>>>> index d741ac5..7f50a33 100644
>>>>>>>> --- a/tools/libacpi/Makefile
>>>>>>>> +++ b/tools/libacpi/Makefile
>>>>>>>> @@ -19,6 +19,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>>>>>>>>
>>>>>>>>  # Sources to be generated
>>>>>>>>  C_SRC = $(addprefix $(ACPI_BUILD_DIR)/, dsdt_anycpu.c dsdt_15cpu.c 
>>>>>>>> dsdt_anycpu_qemu_xen.c dsdt_pvh.c)
>>>>>>>> +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c
>>>>>> Do we really want to generate dsdt_anycpu_arm.c even for x86? Similarly,
>>>>>> do we want to generate x86 dsdt for ARM
>>>> No need I think.
>>>> Boris, will you change this in your next version?
>> You mean adding something along the lines of 'C_SRC-$(CONFIG_X86)'? Yes,
>> it's probably a good idea. I can add that.
> Yes. Then I'll rebase my series on your new version. BTW, when will you
> send out your new series?


I am out till Tuesday (US holiday and a day off), I'll try to post it by
the end of next week (I have v3 ready but need to run it through more tests)

-boris


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