xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64
@ 2016-03-04  6:15 Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables Shannon Zhao
                   ` (21 more replies)
  0 siblings, 22 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

These patches are Part 4 (and last part) of the previous patch set I
sent which adds ACPI support for arm64 on Xen[1]. Split them as an
individual set for convenient reviewing.

These patches create UEFI and ACPI tables which are mapped to Dom0's
space and add other preparations for Dom0 to use ACPI. Then at last
enable ACPI support on ARM64.

See individual patch for changes.

Thanks,
Shannon
[1] http://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01831.html

Naresh Bhat (1):
  xen/arm64: Add ACPI support

Parth Dixit (1):
  arm/p2m: Add helper functions to map memory regions

Shannon Zhao (20):
  arm/acpi: Estimate memory required for acpi/efi tables
  arm/acpi: Add a helper function to get the acpi table offset
  arm/acpi: Prepare FADT table for Dom0
  arm/gic: Add a new callback for creating MADT table for Dom0
  arm/acpi: Prepare MADT table for Dom0
  arm/acpi: Prepare STAO table for Dom0
  arm/acpi: Prepare XSDT table for Dom0
  arm/acpi: Prepare RSDP table for Dom0
  arm/acpi: Map all other tables for Dom0
  arm/acpi: Prepare EFI system table for Dom0
  arm/acpi: Prepare EFI memory descriptor for Dom0
  arm/acpi: Map the new created EFI and ACPI tables to Dom0
  arm/acpi: Create min DT stub for Dom0
  arm/acpi: Permit access all Xen unused SPIs for Dom0
  arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically
  arm/gic: Add a new callback to deny Dom0 access to GIC regions
  arm/acpi: Permit MMIO access of Xen unused devices for Dom0
  hvm/params: Add a new delivery type for event-channel in
    HVM_PARAM_CALLBACK_IRQ
  xen/acpi: Fix event-channel interrupt when booting with ACPI
  xen/arm: Add a hypercall for device mmio mapping

 docs/misc/arm/device-tree/uefi.txt |  58 ++++
 xen/arch/arm/Kconfig               |   9 +
 xen/arch/arm/acpi/lib.c            |  15 +
 xen/arch/arm/domain.c              |   4 +
 xen/arch/arm/domain_build.c        | 588 ++++++++++++++++++++++++++++++++++++-
 xen/arch/arm/efi/Makefile          |   2 +-
 xen/arch/arm/efi/efi-dom0.c        | 187 ++++++++++++
 xen/arch/arm/gic-v2.c              |  65 ++++
 xen/arch/arm/gic-v3.c              |  91 ++++++
 xen/arch/arm/gic.c                 |  10 +
 xen/arch/arm/mm.c                  |   3 +
 xen/arch/arm/p2m.c                 |  49 ++++
 xen/arch/arm/vgic.c                |  38 +++
 xen/common/efi/boot.c              |   7 +
 xen/common/efi/efi.h               |   4 +
 xen/common/efi/runtime.c           |  12 +-
 xen/common/memory.c                |  16 +
 xen/include/asm-arm/acpi.h         |   6 +
 xen/include/asm-arm/config.h       |   4 +
 xen/include/asm-arm/gic.h          |   6 +
 xen/include/asm-arm/p2m.h          |  15 +
 xen/include/asm-arm/setup.h        |  12 +
 xen/include/public/hvm/params.h    |  10 +
 xen/include/public/memory.h        |   1 +
 24 files changed, 1205 insertions(+), 7 deletions(-)
 create mode 100644 docs/misc/arm/device-tree/uefi.txt
 create mode 100644 xen/arch/arm/efi/efi-dom0.c

-- 
2.0.4



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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 10:09   ` Jan Beulich
  2016-03-04  6:15 ` [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset Shannon Zhao
                   ` (20 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, Jan Beulich, zhaoshenglong

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

Estimate the memory required for loading acpi/efi tables in Dom0. Make
the length of each table aligned with 64bit. Alloc the pages to store
the new created EFI and ACPI tables and free these pages when
destroying domain.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: add a new file efi-dom0.c
---
 xen/arch/arm/domain.c       |  4 +++
 xen/arch/arm/domain_build.c | 81 ++++++++++++++++++++++++++++++++++++++++++++-
 xen/arch/arm/efi/Makefile   |  2 +-
 xen/arch/arm/efi/efi-dom0.c | 48 +++++++++++++++++++++++++++
 xen/common/efi/boot.c       |  7 ++++
 xen/common/efi/efi.h        |  4 +++
 xen/include/asm-arm/setup.h |  2 ++
 7 files changed, 146 insertions(+), 2 deletions(-)
 create mode 100644 xen/arch/arm/efi/efi-dom0.c

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 3d274ae..1365b4a 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -640,6 +640,10 @@ void arch_domain_destroy(struct domain *d)
     domain_vgic_free(d);
     domain_vuart_free(d);
     free_xenheap_page(d->shared_info);
+#ifdef CONFIG_ACPI
+    free_xenheap_pages(d->arch.efi_acpi_table,
+                       get_order_from_bytes(d->arch.efi_acpi_len));
+#endif
 }
 
 void arch_domain_shutdown(struct domain *d)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 83676e4..4e20499 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -12,6 +12,8 @@
 #include <xen/libfdt/libfdt.h>
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
+#include <xen/acpi.h>
+#include <acpi/actables.h>
 #include <asm/device.h>
 #include <asm/setup.h>
 #include <asm/platform.h>
@@ -1354,6 +1356,79 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
     return -EINVAL;
 }
 
+#ifdef CONFIG_ACPI
+static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
+{
+    size_t efi_size, acpi_size, madt_size;
+    u64 addr;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_header *table;
+
+    efi_size = estimate_efi_size(kinfo->mem.nr_banks);
+
+    acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8);
+    acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8);
+
+    madt_size = sizeof(struct acpi_table_madt)
+                + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus
+                + sizeof(struct acpi_madt_generic_distributor);
+    if ( d->arch.vgic.version == GIC_V3 )
+        madt_size += sizeof(struct acpi_madt_generic_redistributor)
+                     * d->arch.vgic.nr_regions;
+    acpi_size += ROUNDUP(madt_size, 8);
+
+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+    /* Add place for STAO table in XSDT table */
+    acpi_size += ROUNDUP(table->length + sizeof(u64), 8);
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    acpi_size += ROUNDUP(sizeof(struct acpi_table_rsdp), 8);
+    d->arch.efi_acpi_len = ROUNDUP(efi_size, 8) + ROUNDUP(acpi_size, 8);
+
+    return 0;
+}
+
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    int rc = 0;
+    int order;
+
+    rc = estimate_acpi_efi_size(d, kinfo);
+    if ( rc != 0 )
+        return rc;
+
+    order = get_order_from_bytes(d->arch.efi_acpi_len);
+    d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+    if ( d->arch.efi_acpi_table == NULL )
+    {
+        printk("unable to allocate memory!\n");
+        return -ENOMEM;
+    }
+    memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+    /* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+     * region. So we use it as the ACPI table mapped address. */
+    d->arch.efi_acpi_gpa = kinfo->gnttab_start;
+
+    return 0;
+}
+#else
+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+    /* Only booting with ACPI will hit here */
+    BUG_ON(1);
+    return -EINVAL;
+}
+#endif
 static void dtb_load(struct kernel_info *kinfo)
 {
     void * __user dtb_virt = (void * __user)(register_t)kinfo->dtb_paddr;
@@ -1540,7 +1615,11 @@ int construct_dom0(struct domain *d)
     allocate_memory(d, &kinfo);
     find_gnttab_region(d, &kinfo);
 
-    rc = prepare_dtb(d, &kinfo);
+    if ( acpi_disabled )
+        rc = prepare_dtb(d, &kinfo);
+    else
+        rc = prepare_acpi(d, &kinfo);
+
     if ( rc < 0 )
         return rc;
 
diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
index 729e53e..b38a0c9 100644
--- a/xen/arch/arm/efi/Makefile
+++ b/xen/arch/arm/efi/Makefile
@@ -1,3 +1,3 @@
 CFLAGS += -fshort-wchar
 
-obj-y +=  boot.init.o runtime.o
+obj-y +=  boot.init.o runtime.o efi-dom0.init.o
diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
new file mode 100644
index 0000000..d54c38f
--- /dev/null
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -0,0 +1,48 @@
+/*
+ *  efi-dom0.c - Domain0 EFI Boot Support
+ *
+ *  Copyright (C) 2016 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 General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  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 General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; If not, see <http://www.gnu.org/licenses/>.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include "efi.h"
+#include <asm/setup.h>
+#include <asm/acpi.h>
+
+/* Constant to indicate "Xen" in unicode u16 format */
+static const CHAR16 xen_efi_fw_vendor[] = {0x0058, 0x0065, 0x006E, 0x0000};
+
+size_t __init estimate_efi_size(int mem_nr_banks)
+{
+    size_t size;
+    size_t est_size = sizeof(EFI_SYSTEM_TABLE);
+    size_t ect_size = sizeof(EFI_CONFIGURATION_TABLE);
+    size_t emd_size = sizeof(EFI_MEMORY_DESCRIPTOR);
+    size_t fw_vendor_size = sizeof(xen_efi_fw_vendor);
+    int acpi_mem_nr_banks = 0;
+
+    if ( !acpi_disabled )
+        acpi_mem_nr_banks = get_acpi_meminfo()->nr_banks;
+
+    size = ROUNDUP(est_size + ect_size + fw_vendor_size, 8);
+    /* plus 1 for new created tables */
+    size += ROUNDUP(emd_size * (mem_nr_banks + acpi_mem_nr_banks + 1), 8);
+
+    return size;
+}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 125c9ce..f4f436f 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     for( ; ; ); /* not reached */
 }
 
+#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
+struct meminfo __init *get_acpi_meminfo(void)
+{
+    return &acpi_mem;
+}
+#endif
+
 #ifndef CONFIG_ARM /* TODO - runtime service support */
 
 static bool_t __initdata efi_rs_enable = 1;
diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h
index c557104..d7b1018 100644
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -39,3 +39,7 @@ extern UINT64 efi_boot_max_var_store_size, efi_boot_remain_var_store_size,
 
 unsigned long efi_rs_enter(void);
 void efi_rs_leave(unsigned long);
+
+#ifdef CONFIG_ARM
+struct meminfo *get_acpi_meminfo(void);
+#endif
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 30ac53b..7f233a1 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -51,6 +51,8 @@ void arch_init_memory(void);
 
 void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
+size_t estimate_efi_size(int mem_nr_banks);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 10:59   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 03/22] arm/acpi: Prepare FADT table for Dom0 Shannon Zhao
                   ` (19 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

These tables are aligned with 64bit.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: make the return value type and variable type consistent
---
 xen/arch/arm/acpi/lib.c    | 15 +++++++++++++++
 xen/include/asm-arm/acpi.h |  6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
index db5c4d8..79f7edd 100644
--- a/xen/arch/arm/acpi/lib.c
+++ b/xen/arch/arm/acpi/lib.c
@@ -60,3 +60,18 @@ bool_t __init acpi_psci_hvc_present(void)
 {
     return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
 }
+
+paddr_t __init acpi_get_table_offset(struct membank tbl_add[],
+                                     EFI_MEM_RES index)
+{
+    int i;
+    paddr_t offset = 0;
+
+    for ( i = 0; i < index; i++ )
+    {
+        /* Aligned with 64bit (8 bytes) */
+        offset += ROUNDUP(tbl_add[i].size, 8);
+    }
+
+    return offset;
+}
diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
index 7f59761..6db3711 100644
--- a/xen/include/asm-arm/acpi.h
+++ b/xen/include/asm-arm/acpi.h
@@ -25,6 +25,7 @@
 
 #include <xen/init.h>
 #include <asm/page.h>
+#include <asm/setup.h>
 
 #define COMPILER_DEPENDENT_INT64   long long
 #define COMPILER_DEPENDENT_UINT64  unsigned long long
@@ -58,10 +59,15 @@ static inline void enable_acpi(void)
 {
     acpi_disabled = 0;
 }
+
+paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index);
 #else
 #define acpi_disabled (1)
 #define disable_acpi()
 #define enable_acpi()
+paddr_t inline acpi_get_table_offset(struct membank tbl_add[],
+                                     EFI_MEM_RES index)
+{ return 0; }
 #endif
 
 #endif /*_ASM_ARM_ACPI_H*/
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 03/22] arm/acpi: Prepare FADT table for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 04/22] arm/gic: Add a new callback for creating MADT " Shannon Zhao
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Copy and modify FADT table before passing it to Dom0. Set PSCI_COMPLIANT
and PSCI_USE_HVC.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 4e20499..d9b7213 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,44 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_ACPI
+
+static int acpi_create_fadt(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_fadt *fadt = NULL;
+    u64 table_size;
+    acpi_status status;
+    u8 *base_ptr;
+    u8 checksum;
+
+    status = acpi_get_table(ACPI_SIG_FADT, 0, &table);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        const char *msg = acpi_format_exception(status);
+
+        printk("Failed to get FADT table, %s\n", msg);
+        return -EINVAL;
+    }
+
+    table_size = table->length;
+    base_ptr = d->arch.efi_acpi_table
+               + acpi_get_table_offset(tbl_add, TBL_FADT);
+    ACPI_MEMCPY(base_ptr, table, table_size);
+    fadt = (struct acpi_table_fadt *)base_ptr;
+
+    /* Set PSCI_COMPLIANT and PSCI_USE_HVC */
+    fadt->arm_boot_flags |= (ACPI_FADT_PSCI_COMPLIANT | ACPI_FADT_PSCI_USE_HVC);
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, fadt), table_size);
+    fadt->header.checksum -= checksum;
+
+    tbl_add[TBL_FADT].start = d->arch.efi_acpi_gpa
+                              + acpi_get_table_offset(tbl_add, TBL_FADT);
+    tbl_add[TBL_FADT].size = table_size;
+
+    return 0;
+}
+
 static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
 {
     size_t efi_size, acpi_size, madt_size;
@@ -1401,6 +1439,7 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
 {
     int rc = 0;
     int order;
+    struct membank tbl_add[TBL_MMAX] = {};
 
     rc = estimate_acpi_efi_size(d, kinfo);
     if ( rc != 0 )
@@ -1419,6 +1458,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
      * region. So we use it as the ACPI table mapped address. */
     d->arch.efi_acpi_gpa = kinfo->gnttab_start;
 
+    rc = acpi_create_fadt(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 04/22] arm/gic: Add a new callback for creating MADT table for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (2 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 03/22] arm/acpi: Prepare FADT table for Dom0 Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 05/22] arm/acpi: Prepare " Shannon Zhao
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Add a new member in gic_hw_operations which is used to creat MADT table
for Dom0.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic-v2.c     | 34 ++++++++++++++++++++++++++++++++++
 xen/arch/arm/gic-v3.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/gic.c        |  5 +++++
 xen/include/asm-arm/gic.h |  3 +++
 4 files changed, 89 insertions(+)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 0fcb894..02db5f2 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -685,6 +685,35 @@ static void __init gicv2_dt_init(void)
 }
 
 #ifdef CONFIG_ACPI
+static u32 gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+    struct acpi_subtable_header *header;
+    struct acpi_madt_generic_interrupt *host_gicc, *gicc;
+    u32 i, size, table_len = 0;
+    u8 *base_ptr = d->arch.efi_acpi_table + offset;
+
+    header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+    if ( !header )
+        panic("Can't get GICC entry");
+    host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
+                             header);
+
+    size = sizeof(struct acpi_madt_generic_interrupt);
+    /* Add Generic Interrupt */
+    for ( i = 0; i < d->max_vcpus; i++ )
+    {
+        gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
+        ACPI_MEMCPY(gicc, host_gicc, size);
+        gicc->cpu_interface_number = i;
+        gicc->uid = i;
+        gicc->flags = ACPI_MADT_ENABLED;
+        gicc->arm_mpidr = vcpuid_to_vaffinity(i);
+        table_len += size;
+    }
+
+    return table_len;
+}
+
 static int __init
 gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
                         const unsigned long end)
@@ -776,6 +805,10 @@ static void __init gicv2_acpi_init(void)
 }
 #else
 static void __init gicv2_acpi_init(void) { }
+static u32 gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+    return 0;
+}
 #endif
 
 static int __init gicv2_init(void)
@@ -868,6 +901,7 @@ const static struct gic_hw_operations gicv2_ops = {
     .read_vmcr_priority  = gicv2_read_vmcr_priority,
     .read_apr            = gicv2_read_apr,
     .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
+    .make_hwdom_madt     = gicv2_make_hwdom_madt,
 };
 
 /* Set up the GIC */
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index f83fd88..d9fce4b 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1236,6 +1236,48 @@ static void __init gicv3_dt_init(void)
 }
 
 #ifdef CONFIG_ACPI
+static u32 gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+    struct acpi_subtable_header *header;
+    struct acpi_madt_generic_interrupt *host_gicc, *gicc;
+    struct acpi_madt_generic_redistributor *gicr;
+    u8 *base_ptr = d->arch.efi_acpi_table + offset;
+    u32 i, table_len = 0, size;
+
+    /* Add Generic Interrupt */
+    header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+    if ( !header )
+        panic("Can't get GICC entry");
+    host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
+                             header);
+
+    size = sizeof(struct acpi_madt_generic_interrupt);
+    for ( i = 0; i < d->max_vcpus; i++ )
+    {
+        gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
+        ACPI_MEMCPY(gicc, host_gicc, size);
+        gicc->cpu_interface_number = i;
+        gicc->uid = i;
+        gicc->flags = ACPI_MADT_ENABLED;
+        gicc->arm_mpidr = vcpuid_to_vaffinity(i);
+        table_len += size;
+    }
+
+    /* Add Generic Redistributor */
+    size = sizeof(struct acpi_madt_generic_redistributor);
+    for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
+    {
+        gicr = (struct acpi_madt_generic_redistributor *)(base_ptr + table_len);
+        gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
+        gicr->header.length = size;
+        gicr->base_address = d->arch.vgic.rdist_regions[i].base;
+        gicr->length = d->arch.vgic.rdist_regions[i].size;
+        table_len += size;
+    }
+
+    return table_len;
+}
+
 static int __init
 gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
                         const unsigned long end)
@@ -1380,6 +1422,10 @@ static void __init gicv3_acpi_init(void)
 }
 #else
 static void __init gicv3_acpi_init(void) { }
+static u32 gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+    return 0;
+}
 #endif
 
 /* Set up the GIC */
@@ -1474,6 +1520,7 @@ static const struct gic_hw_operations gicv3_ops = {
     .read_apr            = gicv3_read_apr,
     .secondary_init      = gicv3_secondary_cpu_init,
     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
+    .make_hwdom_madt     = gicv3_make_hwdom_madt,
 };
 
 static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index fbbe37f..6d32432 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -739,6 +739,11 @@ int gic_make_hwdom_dt_node(const struct domain *d,
     return gic_hw_ops->make_hwdom_dt_node(d, gic, fdt);
 }
 
+u32 gic_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+    return gic_hw_ops->make_hwdom_madt(d, offset);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 7bd06e1..4cf003d 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -358,12 +358,15 @@ struct gic_hw_operations {
     /* Create GIC node for the hardware domain */
     int (*make_hwdom_dt_node)(const struct domain *d,
                               const struct dt_device_node *gic, void *fdt);
+    /* Create MADT table for the hardware domain */
+    u32 (*make_hwdom_madt)(const struct domain *d, u32 offset);
 };
 
 void register_gic_ops(const struct gic_hw_operations *ops);
 int gic_make_hwdom_dt_node(const struct domain *d,
                            const struct dt_device_node *gic,
                            void *fdt);
+u32 gic_make_hwdom_madt(const struct domain *d, u32 offset);
 
 #endif /* __ASSEMBLY__ */
 #endif
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 05/22] arm/acpi: Prepare MADT table for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (3 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 04/22] arm/gic: Add a new callback for creating MADT " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 06/22] arm/acpi: Prepare STAO " Shannon Zhao
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Copy main MADT table contents and distributor subtable from physical
ACPI MADT table. Make other subtables through the callback of
gic_hw_ops.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d9b7213..ed257e0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,52 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_ACPI
+static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_madt *madt = NULL;
+    struct acpi_subtable_header *header;
+    struct acpi_madt_generic_distributor *gicd;
+    u32 table_size = sizeof(struct acpi_table_madt);
+    u32 offset = acpi_get_table_offset(tbl_add, TBL_MADT);
+    acpi_status status;
+    u8 *base_ptr, checksum;
+
+    status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        const char *msg = acpi_format_exception(status);
+
+        printk("Failed to get MADT table, %s\n", msg);
+        return -EINVAL;
+    }
+
+    base_ptr = d->arch.efi_acpi_table + offset;
+    ACPI_MEMCPY(base_ptr, table, table_size);
+
+    /* Add Generic Distributor */
+    header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
+    if ( !header )
+        panic("Can't get GICD entry");
+    gicd = container_of(header, struct acpi_madt_generic_distributor, header);
+    ACPI_MEMCPY(base_ptr + table_size, gicd,
+                sizeof(struct acpi_madt_generic_distributor));
+    table_size += sizeof(struct acpi_madt_generic_distributor);
+
+    /* Add other subtables*/
+    table_size += gic_make_hwdom_madt(d, offset + table_size);
+
+    madt = (struct acpi_table_madt *)base_ptr;
+    madt->header.length = table_size;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, madt), table_size);
+    madt->header.checksum -= checksum;
+
+    tbl_add[TBL_MADT].start = d->arch.efi_acpi_gpa + offset;
+    tbl_add[TBL_MADT].size = table_size;
+
+    return 0;
+}
 
 static int acpi_create_fadt(struct domain *d, struct membank tbl_add[])
 {
@@ -1462,6 +1508,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
+    rc = acpi_create_madt(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 06/22] arm/acpi: Prepare STAO table for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (4 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 05/22] arm/acpi: Prepare " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 07/22] arm/acpi: Prepare XSDT " Shannon Zhao
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Create STAO table for Dom0. This table is used to tell Dom0 whether it
should ignore UART defined in SPCR table or the ACPI namespace names.

Look at below url for details:
http://wiki.xenproject.org/mediawiki/images/0/02/Status-override-table.pdf

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ed257e0..b369f2e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,43 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_ACPI
+static int acpi_create_stao(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_stao *stao = NULL;
+    u32 table_size = sizeof(struct acpi_table_stao);
+    u32 offset = acpi_get_table_offset(tbl_add, TBL_STAO);
+    acpi_status status;
+    u8 *base_ptr, checksum;
+
+    /* Copy OEM and ASL compiler fields from another table, use MADT */
+    status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        const char *msg = acpi_format_exception(status);
+
+        printk("Failed to get MADT table, %s\n", msg);
+        return -EINVAL;
+    }
+
+    base_ptr = d->arch.efi_acpi_table + offset;
+    ACPI_MEMCPY(base_ptr, table, sizeof(struct acpi_table_header));
+
+    stao = (struct acpi_table_stao *)base_ptr;
+    ACPI_MEMCPY(stao->header.signature, ACPI_SIG_STAO, 4);
+    stao->header.revision = 1;
+    stao->header.length = table_size;
+    stao->ignore_uart = 1;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, stao), table_size);
+    stao->header.checksum -= checksum;
+
+    tbl_add[TBL_STAO].start = d->arch.efi_acpi_gpa + offset;
+    tbl_add[TBL_STAO].size = table_size;
+
+    return 0;
+}
+
 static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
 {
     struct acpi_table_header *table = NULL;
@@ -1512,6 +1549,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
+    rc = acpi_create_stao(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 07/22] arm/acpi: Prepare XSDT table for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (5 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 06/22] arm/acpi: Prepare STAO " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 08/22] arm/acpi: Prepare RSDP " Shannon Zhao
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Copy and modify XSDT table before passing it to Dom0. Repalce the entry
value of the copied table. Add a new entry for STAO table as well. And
keep entry value of other reused tables unchanged.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 72 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index b369f2e..f9fe289 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,74 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_ACPI
+static void acpi_xsdt_modify_entry(u64 entry[], unsigned long entry_count,
+                                   char *signature, u64 addr)
+{
+    int i;
+    struct acpi_table_header *table;
+    u64 size = sizeof(struct acpi_table_header);
+
+    for( i = 0; i < entry_count; i++ )
+    {
+        table = acpi_os_map_memory(entry[i], size);
+        if ( ACPI_COMPARE_NAME(table->signature, signature) )
+        {
+            entry[i] = addr;
+            acpi_os_unmap_memory(table, size);
+            break;
+        }
+        acpi_os_unmap_memory(table, size);
+    }
+}
+
+static int acpi_create_xsdt(struct domain *d, struct membank tbl_add[])
+{
+    struct acpi_table_header *table = NULL;
+    struct acpi_table_rsdp *rsdp_tbl;
+    struct acpi_table_xsdt *xsdt = NULL;
+    u64 table_size, addr;
+    unsigned long entry_count;
+    u8 *base_ptr;
+    u8 checksum;
+
+    addr = acpi_os_get_root_pointer();
+    if ( !addr )
+    {
+        printk("Unable to get acpi root pointer\n");
+        return -EINVAL;
+    }
+    rsdp_tbl = acpi_os_map_memory(addr, sizeof(struct acpi_table_rsdp));
+    table = acpi_os_map_memory(rsdp_tbl->xsdt_physical_address,
+                               sizeof(struct acpi_table_header));
+
+    /* Add place for STAO table in XSDT table */
+    table_size = table->length + sizeof(u64);
+    entry_count = (table->length - sizeof(struct acpi_table_header))
+                  / sizeof(u64);
+    base_ptr = d->arch.efi_acpi_table
+               + acpi_get_table_offset(tbl_add, TBL_XSDT);
+    ACPI_MEMCPY(base_ptr, table, table->length);
+    acpi_os_unmap_memory(table, sizeof(struct acpi_table_header));
+    acpi_os_unmap_memory(rsdp_tbl, sizeof(struct acpi_table_rsdp));
+
+    xsdt = (struct acpi_table_xsdt *)base_ptr;
+    acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
+                           ACPI_SIG_FADT, tbl_add[TBL_FADT].start);
+    acpi_xsdt_modify_entry(xsdt->table_offset_entry, entry_count,
+                           ACPI_SIG_MADT, tbl_add[TBL_MADT].start);
+    xsdt->table_offset_entry[entry_count] = tbl_add[TBL_STAO].start;
+
+    xsdt->header.length = table_size;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, xsdt), table_size);
+    xsdt->header.checksum -= checksum;
+
+    tbl_add[TBL_XSDT].start = d->arch.efi_acpi_gpa
+                              + acpi_get_table_offset(tbl_add, TBL_XSDT);
+    tbl_add[TBL_XSDT].size = table_size;
+
+    return 0;
+}
+
 static int acpi_create_stao(struct domain *d, struct membank tbl_add[])
 {
     struct acpi_table_header *table = NULL;
@@ -1553,6 +1621,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
+    rc = acpi_create_xsdt(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 08/22] arm/acpi: Prepare RSDP table for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (6 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 07/22] arm/acpi: Prepare XSDT " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 09/22] arm/p2m: Add helper functions to map memory regions Shannon Zhao
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Copy RSDP table and replace rsdp->xsdt_physical_address with new address
of XSDT table, so it can point to the right XSDT table.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f9fe289..ea7d6a5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,38 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_ACPI
+static int acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
+{
+
+    struct acpi_table_rsdp *rsdp = NULL;
+    u64 addr;
+    u64 table_size = sizeof(struct acpi_table_rsdp);
+    u8 *base_ptr;
+    u8 checksum;
+
+    addr = acpi_os_get_root_pointer();
+    if( !addr )
+        panic("Unable to get acpi root pointer\n");
+
+    rsdp = acpi_os_map_memory(addr, table_size);
+    base_ptr = d->arch.efi_acpi_table
+               + acpi_get_table_offset(tbl_add, TBL_RSDP);
+    ACPI_MEMCPY(base_ptr, rsdp, table_size);
+    acpi_os_unmap_memory(rsdp, table_size);
+
+    rsdp = (struct acpi_table_rsdp *)base_ptr;
+    /* Replace xsdt_physical_address */
+    rsdp->xsdt_physical_address = tbl_add[TBL_XSDT].start;
+    checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, rsdp), table_size);
+    rsdp->checksum = rsdp->checksum - checksum;
+
+    tbl_add[TBL_RSDP].start = d->arch.efi_acpi_gpa
+                              + acpi_get_table_offset(tbl_add, TBL_RSDP);
+    tbl_add[TBL_RSDP].size = table_size;
+
+    return 0;
+}
+
 static void acpi_xsdt_modify_entry(u64 entry[], unsigned long entry_count,
                                    char *signature, u64 addr)
 {
@@ -1625,6 +1657,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
+    rc = acpi_create_rsdp(d, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 09/22] arm/p2m: Add helper functions to map memory regions
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (7 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 08/22] arm/acpi: Prepare RSDP " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 10:51   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 10/22] arm/acpi: Map all other tables for Dom0 Shannon Zhao
                   ` (12 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

From: Parth Dixit <parth.dixit@linaro.org>

Create a helper function for mapping with cached attributes and
read-write range.

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: use p2m_access_rw and rename to map_regions_rw
---
 xen/arch/arm/p2m.c        | 26 ++++++++++++++++++++++++++
 xen/include/asm-arm/p2m.h | 10 ++++++++++
 2 files changed, 36 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index a2a9c4b..d206616 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1218,6 +1218,32 @@ int p2m_populate_ram(struct domain *d,
                              d->arch.p2m.default_access);
 }
 
+int map_regions_rw(struct domain *d,
+                     unsigned long start_gfn,
+                     unsigned long nr,
+                     unsigned long mfn)
+{
+    return apply_p2m_changes(d, INSERT,
+                             pfn_to_paddr(start_gfn),
+                             pfn_to_paddr(start_gfn + nr),
+                             pfn_to_paddr(mfn),
+                             MATTR_MEM, 0, p2m_mmio_direct,
+                             p2m_access_rw);
+}
+
+int unmap_regions_rw(struct domain *d,
+                       unsigned long start_gfn,
+                       unsigned long nr,
+                       unsigned long mfn)
+{
+    return apply_p2m_changes(d, REMOVE,
+                             pfn_to_paddr(start_gfn),
+                             pfn_to_paddr(start_gfn + nr),
+                             pfn_to_paddr(mfn),
+                             MATTR_MEM, 0, p2m_invalid,
+                             p2m_access_rw);
+}
+
 int map_mmio_regions(struct domain *d,
                      unsigned long start_gfn,
                      unsigned long nr,
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 433952a..17be6ad 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -144,6 +144,16 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
 /* Setup p2m RAM mapping for domain d from start-end. */
 int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
 
+int map_regions_rw(struct domain *d,
+                    unsigned long start_gfn,
+                    unsigned long nr_mfns,
+                    unsigned long mfn);
+
+int unmap_regions_rw(struct domain *d,
+                    unsigned long start_gfn,
+                    unsigned long nr_mfns,
+                    unsigned long mfn);
+
 int guest_physmap_add_entry(struct domain *d,
                             unsigned long gfn,
                             unsigned long mfn,
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 10/22] arm/acpi: Map all other tables for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (8 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 09/22] arm/p2m: Add helper functions to map memory regions Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 11/22] arm/acpi: Prepare EFI system table " Shannon Zhao
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Map all other tables to Dom0 using 1:1 mappings.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ea7d6a5..c71976c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,30 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_ACPI
+static void acpi_map_other_tables(struct domain *d)
+{
+    int i;
+    unsigned long res;
+    u64 addr, size;
+
+    /* Map all other tables to Dom0 using 1:1 mappings. */
+    for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
+    {
+        addr = acpi_gbl_root_table_list.tables[i].address;
+        size = acpi_gbl_root_table_list.tables[i].length;
+        res = map_regions_rw(d,
+                             paddr_to_pfn(addr & PAGE_MASK),
+                             DIV_ROUND_UP(size, PAGE_SIZE),
+                             paddr_to_pfn(addr & PAGE_MASK));
+        if ( res )
+        {
+             panic(XENLOG_ERR "Unable to map 0x%"PRIx64
+                   " - 0x%"PRIx64" in domain \n",
+                   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
+        }
+    }
+}
+
 static int acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
 {
 
@@ -1661,6 +1685,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
+    acpi_map_other_tables(d);
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 11/22] arm/acpi: Prepare EFI system table for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (9 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 10/22] arm/acpi: Map all other tables for Dom0 Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 11:02   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 12/22] arm/acpi: Prepare EFI memory descriptor " Shannon Zhao
                   ` (10 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Prepare EFI system table for Dom0 to describe the information of UEFI.

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: move to efi-dom0.c
---
 xen/arch/arm/domain_build.c |  2 ++
 xen/arch/arm/efi/efi-dom0.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/setup.h |  3 +++
 3 files changed, 49 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c71976c..613551c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1686,6 +1686,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
         return rc;
 
     acpi_map_other_tables(d);
+    acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
+                                 tbl_add);
 
     return 0;
 }
diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index d54c38f..36a1283 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -46,3 +46,47 @@ size_t __init estimate_efi_size(int mem_nr_banks)
 
     return size;
 }
+
+#include "../../../common/decompress.h"
+#define XZ_EXTERN STATIC
+#include "../../../common/xz/crc32.c"
+
+void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
+                                         struct membank tbl_add[])
+{
+    u64 table_addr, table_size, offset = 0;
+    u8 *base_ptr;
+    EFI_CONFIGURATION_TABLE *efi_conf_tbl;
+    EFI_SYSTEM_TABLE *efi_sys_tbl;
+
+    table_addr = paddr + acpi_get_table_offset(tbl_add, TBL_EFIT);
+    table_size = sizeof(EFI_SYSTEM_TABLE) + sizeof(EFI_CONFIGURATION_TABLE)
+                 + sizeof(xen_efi_fw_vendor);
+    base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_EFIT);
+    efi_sys_tbl = (EFI_SYSTEM_TABLE *)base_ptr;
+
+    efi_sys_tbl->Hdr.Signature = EFI_SYSTEM_TABLE_SIGNATURE;
+    /* Specify the revision as 2.5 */
+    efi_sys_tbl->Hdr.Revision = (2 << 16 | 50);
+    efi_sys_tbl->Hdr.HeaderSize = table_size;
+
+    efi_sys_tbl->FirmwareRevision = 1;
+    efi_sys_tbl->NumberOfTableEntries = 1;
+    offset += sizeof(EFI_SYSTEM_TABLE);
+    memcpy((CHAR16 *)(base_ptr + offset), xen_efi_fw_vendor,
+           sizeof(xen_efi_fw_vendor));
+    efi_sys_tbl->FirmwareVendor = (CHAR16 *)(table_addr + offset);
+
+    offset += sizeof(xen_efi_fw_vendor);
+    efi_conf_tbl = (EFI_CONFIGURATION_TABLE *)(base_ptr + offset);
+    efi_conf_tbl->VendorGuid = (EFI_GUID)ACPI_20_TABLE_GUID;
+    efi_conf_tbl->VendorTable = (VOID *)tbl_add[TBL_RSDP].start;
+    efi_sys_tbl->ConfigurationTable = (EFI_CONFIGURATION_TABLE *)(table_addr
+                                                                  + offset);
+    xz_crc32_init();
+    efi_sys_tbl->Hdr.CRC32 = xz_crc32((uint8_t *)efi_sys_tbl,
+                                      efi_sys_tbl->Hdr.HeaderSize, 0);
+
+    tbl_add[TBL_EFIT].start = table_addr;
+    tbl_add[TBL_EFIT].size = table_size;
+}
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index 7f233a1..e423b15 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -53,6 +53,9 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
 
 size_t estimate_efi_size(int mem_nr_banks);
 
+void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
+                                  struct membank tbl_add[]);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (10 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 11/22] arm/acpi: Prepare EFI system table " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 11:13   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0 Shannon Zhao
                   ` (9 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Create a few EFI memory descriptors to tell Dom0 the RAM region
information, ACPI table regions and EFI tables reserved resions.

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: move to efi-dom0.c
---
 xen/arch/arm/domain_build.c |  2 ++
 xen/arch/arm/efi/efi-dom0.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/setup.h |  5 +++++
 3 files changed, 54 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 613551c..008fc76 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     acpi_map_other_tables(d);
     acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
                                  tbl_add);
+    acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
+                               d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
 
     return 0;
 }
diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index 36a1283..0ff6309 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -22,6 +22,7 @@
  */
 
 #include "efi.h"
+#include <xen/pfn.h>
 #include <asm/setup.h>
 #include <asm/acpi.h>
 
@@ -90,3 +91,49 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
     tbl_add[TBL_EFIT].start = table_addr;
     tbl_add[TBL_EFIT].size = table_size;
 }
+
+void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
+                                       void *efi_acpi_table,
+                                       const struct meminfo *mem,
+                                       struct membank tbl_add[])
+{
+    EFI_MEMORY_DESCRIPTOR *memory_map;
+    struct meminfo *acpi_mem;
+    int acpi_mem_nr_banks = 0;
+    unsigned int i, offset;
+    u8 *base_ptr;
+
+    base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP);
+    memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr);
+
+    offset = 0;
+    for( i = 0; i < mem->nr_banks; i++, offset++ )
+    {
+        memory_map[offset].Type = EfiConventionalMemory;
+        memory_map[offset].PhysicalStart = mem->bank[i].start;
+        memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size);
+        memory_map[offset].Attribute = EFI_MEMORY_WB;
+    }
+
+    if ( !acpi_disabled )
+    {
+        acpi_mem = get_acpi_meminfo();
+        acpi_mem_nr_banks = acpi_mem->nr_banks;
+        for( i = 0; i < acpi_mem_nr_banks; i++, offset++ )
+        {
+            memory_map[offset].Type = EfiACPIReclaimMemory;
+            memory_map[offset].PhysicalStart = acpi_mem->bank[i].start;
+            memory_map[offset].NumberOfPages = PFN_UP(acpi_mem->bank[i].size);
+            memory_map[offset].Attribute = EFI_MEMORY_WB;
+        }
+    }
+
+    memory_map[offset].Type = EfiACPIReclaimMemory;
+    memory_map[offset].PhysicalStart = paddr;
+    memory_map[offset].NumberOfPages = PFN_UP(size);
+    memory_map[offset].Attribute = EFI_MEMORY_WB;
+
+    tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
+    tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
+                             * (mem->nr_banks + acpi_mem_nr_banks + 1);
+}
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index e423b15..b2899f2 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -56,6 +56,11 @@ size_t estimate_efi_size(int mem_nr_banks);
 void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
                                   struct membank tbl_add[]);
 
+void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
+                                void *efi_acpi_table,
+                                const struct meminfo *mem,
+                                struct membank tbl_add[]);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (11 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 12/22] arm/acpi: Prepare EFI memory descriptor " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 14/22] arm/acpi: Create min DT stub for Dom0 Shannon Zhao
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 008fc76..e036887 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
                                d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
 
+    /* Map the EFI and ACPI tables to Dom0 */
+    rc = map_regions_rw(d,
+                        paddr_to_pfn(d->arch.efi_acpi_gpa),
+                        PFN_UP(d->arch.efi_acpi_len),
+                        paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));
+    if ( rc != 0 )
+    {
+        printk(XENLOG_ERR "Unable to map 0x%"PRIx64
+               " - 0x%"PRIx64" in domain %d\n",
+               d->arch.efi_acpi_gpa & PAGE_MASK,
+               PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
+               d->domain_id);
+        return rc;
+    }
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 14/22] arm/acpi: Create min DT stub for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (12 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0 Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 11:17   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 15/22] arm/acpi: Permit access all Xen unused SPIs " Shannon Zhao
                   ` (7 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Create a DT for Dom0 for ACPI-case only. DT contains minimal required
informations such as Dom0 bootargs, initrd, efi description table and
address of uefi memory table.

Also port the document of this device tree bindings from Linux.

Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: change the file name
---
 docs/misc/arm/device-tree/uefi.txt |  58 +++++++++++++++
 xen/arch/arm/domain_build.c        | 143 +++++++++++++++++++++++++++++++++++++
 xen/arch/arm/efi/efi-dom0.c        |  48 +++++++++++++
 xen/include/asm-arm/setup.h        |   2 +
 4 files changed, 251 insertions(+)
 create mode 100644 docs/misc/arm/device-tree/uefi.txt

diff --git a/docs/misc/arm/device-tree/uefi.txt b/docs/misc/arm/device-tree/uefi.txt
new file mode 100644
index 0000000..41a8be0
--- /dev/null
+++ b/docs/misc/arm/device-tree/uefi.txt
@@ -0,0 +1,58 @@
+* Xen hypervisor device tree bindings
+
+Xen ARM virtual platforms shall have a top-level "hypervisor" node with
+the following properties:
+
+- compatible:
+	compatible = "xen,xen-<version>", "xen,xen";
+  where <version> is the version of the Xen ABI of the platform.
+
+- reg: specifies the base physical address and size of a region in
+  memory where the grant table should be mapped to, using an
+  HYPERVISOR_memory_op hypercall. The memory region is large enough to map
+  the whole grant table (it is larger or equal to gnttab_max_grant_frames()).
+
+- interrupts: the interrupt used by Xen to inject event notifications.
+  A GIC node is also required.
+
+To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
+under /hypervisor with following parameters:
+
+________________________________________________________________________________
+Name                      | Size   | Description
+================================================================================
+xen,uefi-system-table     | 64-bit | Guest physical address of the UEFI System
+                          |        | Table.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-start       | 64-bit | Guest physical address of the UEFI memory
+                          |        | map.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
+                          |        | pointed to in previous entry.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
+                          |        | memory map.
+--------------------------------------------------------------------------------
+xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
+--------------------------------------------------------------------------------
+
+Example (assuming #address-cells = <2> and #size-cells = <2>):
+
+hypervisor {
+	compatible = "xen,xen-4.3", "xen,xen";
+	reg = <0 0xb0000000 0 0x20000>;
+	interrupts = <1 15 0xf08>;
+	uefi {
+		xen,uefi-system-table = <0xXXXXXXXX>;
+		xen,uefi-mmap-start = <0xXXXXXXXX>;
+		xen,uefi-mmap-size = <0xXXXXXXXX>;
+		xen,uefi-mmap-desc-size = <0xXXXXXXXX>;
+		xen,uefi-mmap-desc-ver = <0xXXXXXXXX>;
+        };
+};
+
+The format and meaning of the "xen,uefi-*" parameters are similar to those in
+Documentation/arm/uefi.txt in Linux, which are provided by the regular Linux
+UEFI stub. However they differ because they are provided by the Xen hypervisor,
+together with a set of UEFI runtime services implemented via hypercalls, see
+xen/include/public/platform.h.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e036887..6726e45 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,145 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 }
 
 #ifdef CONFIG_ACPI
+#define ACPI_DOM0_FDT_MIN_SIZE 4096
+
+static int make_chosen_node(const struct kernel_info *kinfo,
+                            struct membank tbl_add[])
+{
+    int res;
+    const char *bootargs = NULL;
+    const struct bootmodule *mod = kinfo->kernel_bootmodule;
+    void *fdt = kinfo->fdt;
+
+    DPRINT("Create chosen node\n");
+    res = fdt_begin_node(fdt, "chosen");
+    if ( res )
+        return res;
+
+    if ( mod && mod->cmdline[0] )
+    {
+        bootargs = &mod->cmdline[0];
+        res = fdt_property(fdt, "bootargs", bootargs, strlen(bootargs) + 1);
+        if ( res )
+           return res;
+    }
+
+    /*
+     * If the bootloader provides an initrd, we must create a placeholder
+     * for the initrd properties. The values will be replaced later.
+     */
+    if ( mod && mod->size )
+    {
+        u64 a = 0;
+        res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a));
+        if ( res )
+            return res;
+
+        res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a));
+        if ( res )
+            return res;
+    }
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+static int acpi_make_hypervisor_node(const struct kernel_info *kinfo,
+                                     struct membank tbl_add[])
+{
+    const char compat[] =
+        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
+        "xen,xen";
+    int res;
+    /* Convenience alias */
+    void *fdt = kinfo->fdt;
+
+    DPRINT("Create hypervisor node\n");
+
+    /* See linux Documentation/devicetree/bindings/arm/xen.txt */
+    res = fdt_begin_node(fdt, "hypervisor");
+    if ( res )
+        return res;
+
+    /* Cannot use fdt_property_string due to embedded nulls */
+    res = fdt_property(fdt, "compatible", compat, sizeof(compat));
+    if ( res )
+        return res;
+
+    res = arm_acpi_make_efi_nodes(fdt, tbl_add);
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
+
+/*
+ * Prepare a minimal DTB for Dom0 which contains bootargs, initrd, memory
+ * information, EFI table.
+ */
+static int create_acpi_dtb(struct kernel_info *kinfo, struct membank tbl_add[])
+{
+    int new_size;
+    int ret;
+
+    DPRINT("Prepare a min DTB for DOM0\n");
+
+    /* Allocate min size for DT */
+    new_size = ACPI_DOM0_FDT_MIN_SIZE;
+    kinfo->fdt = xmalloc_bytes(new_size);
+
+    if ( kinfo->fdt == NULL )
+        return -ENOMEM;
+
+    /* Create a new empty DT for DOM0 */
+    ret = fdt_create(kinfo->fdt, new_size);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_finish_reservemap(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_begin_node(kinfo->fdt, "/");
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_property_cell(kinfo->fdt, "#address-cells", 2);
+    if ( ret )
+        return ret;
+
+    ret = fdt_property_cell(kinfo->fdt, "#size-cells", 1);
+    if ( ret )
+        return ret;
+
+    /* Create a chosen node for DOM0 */
+    ret = make_chosen_node(kinfo, tbl_add);
+    if ( ret )
+        goto err;
+
+    ret = acpi_make_hypervisor_node(kinfo, tbl_add);
+    if ( ret )
+        goto err;
+
+    ret = fdt_end_node(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    ret = fdt_finish(kinfo->fdt);
+    if ( ret < 0 )
+        goto err;
+
+    return 0;
+
+  err:
+    printk("Device tree generation failed (%d).\n", ret);
+    xfree(kinfo->fdt);
+    return -EINVAL;
+}
+
 static void acpi_map_other_tables(struct domain *d)
 {
     int i;
@@ -1706,6 +1845,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
         return rc;
     }
 
+    rc = create_acpi_dtb(kinfo, tbl_add);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index 0ff6309..3a7b601 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -23,6 +23,7 @@
 
 #include "efi.h"
 #include <xen/pfn.h>
+#include <xen/libfdt/libfdt.h>
 #include <asm/setup.h>
 #include <asm/acpi.h>
 
@@ -137,3 +138,50 @@ void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
     tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
                              * (mem->nr_banks + acpi_mem_nr_banks + 1);
 }
+
+/* Create place holder for efi values. */
+int __init arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[])
+{
+    u64 fdt_val64;
+    u32 fdt_val32;
+    int desc_ver = 1;
+    int res;
+
+    res = fdt_begin_node(fdt, "uefi");
+    if ( res )
+        return res;
+
+    fdt_val64 = cpu_to_fdt64(tbl_add[TBL_EFIT].start);
+    res = fdt_property(fdt, "xen,uefi-system-table",
+                       &fdt_val64, sizeof(fdt_val64));
+    if ( res )
+        return res;
+
+    fdt_val64 = cpu_to_fdt64(tbl_add[TBL_MMAP].start);
+    res = fdt_property(fdt, "xen,uefi-mmap-start",
+                       &fdt_val64,  sizeof(fdt_val64));
+    if ( res )
+        return res;
+
+    fdt_val32 = cpu_to_fdt32(tbl_add[TBL_MMAP].size);
+    res = fdt_property(fdt, "xen,uefi-mmap-size",
+                       &fdt_val32,  sizeof(fdt_val32));
+    if ( res )
+        return res;
+
+    fdt_val32 = cpu_to_fdt32(sizeof(EFI_MEMORY_DESCRIPTOR));
+    res = fdt_property(fdt, "xen,uefi-mmap-desc-size",
+                         &fdt_val32, sizeof(fdt_val32));
+    if ( res )
+        return res;
+
+    fdt_val32 = cpu_to_fdt32(desc_ver);
+    res = fdt_property(fdt, "xen,uefi-mmap-desc-ver",
+                         &fdt_val32, sizeof(fdt_val32));
+    if ( res )
+        return res;
+
+    res = fdt_end_node(fdt);
+
+    return res;
+}
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index b2899f2..05f0210 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -61,6 +61,8 @@ void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
                                 const struct meminfo *mem,
                                 struct membank tbl_add[]);
 
+int arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
+
 int construct_dom0(struct domain *d);
 
 void discard_initial_modules(void);
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 15/22] arm/acpi: Permit access all Xen unused SPIs for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (13 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 14/22] arm/acpi: Create min DT stub for Dom0 Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically Shannon Zhao
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Permit access all Xen unused SPIs for Dom0 except the interrupts that
Xen uses. Then when Dom0 configures the interrupt, it could set the
interrupt type and route it to Dom0.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 6726e45..1e5ee0e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1359,6 +1359,33 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 #ifdef CONFIG_ACPI
 #define ACPI_DOM0_FDT_MIN_SIZE 4096
 
+static int acpi_permit_spi_access(struct domain *d)
+{
+    int i, res;
+    struct irq_desc *desc;
+
+    /* Here just permit Dom0 to access the SPIs which Xen doesn't use. Then when
+     * Dom0 configures the interrupt, set the interrupt type and route it to
+     * Dom0.
+     */
+    for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
+    {
+        desc = irq_to_desc(i);
+        if( desc->action != NULL)
+            continue;
+
+        res = irq_permit_access(d, i);
+        if ( res )
+        {
+            printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
+                   d->domain_id, i);
+            return res;
+        }
+    }
+
+    return 0;
+}
+
 static int make_chosen_node(const struct kernel_info *kinfo,
                             struct membank tbl_add[])
 {
@@ -1849,6 +1876,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
+    rc = acpi_permit_spi_access(d);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (14 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 15/22] arm/acpi: Permit access all Xen unused SPIs " Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 11:26   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions Shannon Zhao
                   ` (5 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Interrupt information is described in DSDT and is not available at the
time of booting. Check if the interrupt is permitted to access and set
the interrupt type, route it to guest dynamically only for SPI
and Dom0.

Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: add a small function to get the irq type from ICFG
---
 xen/arch/arm/vgic.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index ee35683..52378a3 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -25,6 +25,8 @@
 #include <xen/irq.h>
 #include <xen/sched.h>
 #include <xen/perfc.h>
+#include <xen/iocap.h>
+#include <xen/acpi.h>
 
 #include <asm/current.h>
 
@@ -334,6 +336,21 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
     }
 }
 
+#ifdef CONFIG_ACPI
+#define VGIC_ICFG_MASK(intr) ( 1 << ( ( 2 * ( intr % 16 ) ) + 1 ) )
+
+static unsigned int get_the_irq_type(struct vcpu *v, int n, int index)
+{
+    struct vgic_irq_rank *vr = vgic_get_rank(v, n);
+    uint32_t tr = vr->icfg[index >> 4];
+
+    if ( tr & VGIC_ICFG_MASK(index) )
+        return IRQ_TYPE_EDGE_BOTH;
+    else
+        return IRQ_TYPE_LEVEL_MASK;
+}
+#endif
+
 void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
 {
     const unsigned long mask = r;
@@ -342,9 +359,30 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     unsigned long flags;
     int i = 0;
     struct vcpu *v_target;
+#ifdef CONFIG_ACPI
+    struct domain *d = v->domain;
+    int ret;
+#endif
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
         irq = i + (32 * n);
+#ifdef CONFIG_ACPI
+        /* Set the irq type and route it to guest only for SPI and Dom0 */
+        if( irq_access_permitted(d, irq) && is_hardware_domain(d) &&
+            ( irq >= 32 ) && ( !acpi_disabled ) )
+        {
+            ret = irq_set_spi_type(irq, get_the_irq_type(v, n, i));
+            if ( ret )
+                printk(XENLOG_WARNING "The irq type is not correct\n");
+
+            vgic_reserve_virq(d, irq);
+
+            ret = route_irq_to_guest(d, irq, irq, NULL);
+            if ( ret )
+                printk(XENLOG_ERR "Unable to route IRQ %u to domain %u\n",
+                       irq, d->domain_id);
+        }
+#endif
         v_target = __vgic_get_target_vcpu(v, irq);
         p = irq_to_pending(v_target, irq);
         set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (15 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 11:59   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0 Shannon Zhao
                   ` (4 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Add a new member in gic_hw_operations which is used to deny Dom0 access
to GIC regions.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 xen/arch/arm/gic-v2.c     | 31 +++++++++++++++++++++++++++++++
 xen/arch/arm/gic-v3.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/gic.c        |  5 +++++
 xen/include/asm-arm/gic.h |  3 +++
 4 files changed, 83 insertions(+)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 02db5f2..186f944 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -22,6 +22,7 @@
 #include <xen/init.h>
 #include <xen/mm.h>
 #include <xen/irq.h>
+#include <xen/iocap.h>
 #include <xen/sched.h>
 #include <xen/errno.h>
 #include <xen/softirq.h>
@@ -714,6 +715,31 @@ static u32 gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
     return table_len;
 }
 
+static int gicv2_iomem_deny_access(const struct domain *d)
+{
+    int rc;
+    unsigned long gfn, nr;
+
+    gfn = dbase >> PAGE_SHIFT;
+    rc = iomem_deny_access(d, gfn, gfn + 1);
+    if ( rc )
+        return rc;
+
+    gfn = hbase >> PAGE_SHIFT;
+    rc = iomem_deny_access(d, gfn, gfn + 1);
+    if ( rc )
+        return rc;
+
+    gfn = cbase >> PAGE_SHIFT;
+    nr = DIV_ROUND_UP(csize, PAGE_SIZE);
+    rc = iomem_deny_access(d, gfn, gfn + nr);
+    if ( rc )
+        return rc;
+
+    gfn = vbase >> PAGE_SHIFT;
+    return iomem_deny_access(d, gfn, gfn + nr);
+}
+
 static int __init
 gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
                         const unsigned long end)
@@ -809,6 +835,10 @@ static u32 gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
 {
     return 0;
 }
+static int gicv2_iomem_deny_access(const struct domain *d)
+{
+    return 0;
+}
 #endif
 
 static int __init gicv2_init(void)
@@ -902,6 +932,7 @@ const static struct gic_hw_operations gicv2_ops = {
     .read_apr            = gicv2_read_apr,
     .make_hwdom_dt_node  = gicv2_make_hwdom_dt_node,
     .make_hwdom_madt     = gicv2_make_hwdom_madt,
+    .iomem_deny_access   = gicv2_iomem_deny_access,
 };
 
 /* Set up the GIC */
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index d9fce4b..67797f2 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -27,6 +27,7 @@
 #include <xen/cpu.h>
 #include <xen/mm.h>
 #include <xen/irq.h>
+#include <xen/iocap.h>
 #include <xen/sched.h>
 #include <xen/errno.h>
 #include <xen/delay.h>
@@ -1278,6 +1279,44 @@ static u32 gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
     return table_len;
 }
 
+static int gicv3_iomem_deny_access(const struct domain *d)
+{
+    int rc, i;
+    unsigned long gfn, nr;
+
+    gfn = dbase >> PAGE_SHIFT;
+    rc = iomem_deny_access(d, gfn, gfn + 1);
+    if ( rc )
+        return rc;
+
+    for ( i = 0; i < gicv3.rdist_count; i++ )
+    {
+        gfn = gicv3.rdist_regions[i].base >> PAGE_SHIFT;
+        nr = DIV_ROUND_UP(gicv3.rdist_regions[i].size, PAGE_SIZE);
+        rc = iomem_deny_access(d, gfn, gfn + nr);
+        if ( rc )
+            return rc;
+    }
+
+    if ( cbase != INVALID_PADDR )
+    {
+        gfn = cbase >> PAGE_SHIFT;
+        nr = DIV_ROUND_UP(csize, PAGE_SIZE);
+        rc = iomem_deny_access(d, gfn, gfn + nr);
+        if ( rc )
+            return rc;
+    }
+
+    if ( vbase != INVALID_PADDR )
+    {
+        gfn = vbase >> PAGE_SHIFT;
+        nr = DIV_ROUND_UP(csize, PAGE_SIZE);
+        return iomem_deny_access(d, gfn, gfn + nr);
+    }
+
+    return 0;
+}
+
 static int __init
 gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
                         const unsigned long end)
@@ -1426,6 +1465,10 @@ static u32 gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
 {
     return 0;
 }
+static int gicv3_iomem_deny_access(const struct domain *d)
+{
+    return 0;
+}
 #endif
 
 /* Set up the GIC */
@@ -1521,6 +1564,7 @@ static const struct gic_hw_operations gicv3_ops = {
     .secondary_init      = gicv3_secondary_cpu_init,
     .make_hwdom_dt_node  = gicv3_make_hwdom_dt_node,
     .make_hwdom_madt     = gicv3_make_hwdom_madt,
+    .iomem_deny_access   = gicv3_iomem_deny_access,
 };
 
 static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 6d32432..65022ee 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -744,6 +744,11 @@ u32 gic_make_hwdom_madt(const struct domain *d, u32 offset)
     return gic_hw_ops->make_hwdom_madt(d, offset);
 }
 
+int gic_iomem_deny_access(const struct domain *d)
+{
+    return gic_hw_ops->iomem_deny_access(d);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 4cf003d..932fc02 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -360,6 +360,8 @@ struct gic_hw_operations {
                               const struct dt_device_node *gic, void *fdt);
     /* Create MADT table for the hardware domain */
     u32 (*make_hwdom_madt)(const struct domain *d, u32 offset);
+    /* Deny access to GIC regions */
+    int (*iomem_deny_access)(const struct domain *d);
 };
 
 void register_gic_ops(const struct gic_hw_operations *ops);
@@ -367,6 +369,7 @@ int gic_make_hwdom_dt_node(const struct domain *d,
                            const struct dt_device_node *gic,
                            void *fdt);
 u32 gic_make_hwdom_madt(const struct domain *d, u32 offset);
+int gic_iomem_deny_access(const struct domain *d);
 
 #endif /* __ASSEMBLY__ */
 #endif
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (16 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 12:01   ` Stefano Stabellini
  2016-03-04  6:15 ` [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ Shannon Zhao
                   ` (3 subsequent siblings)
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Firstly it permits full MMIO capabilities for Dom0. Then deny MMIO
access of Xen used devices, such as UART, GIC, SMMU. Currently, it only
denies the MMIO access of UART and GIC regions. For other Xen used
devices it could be added later when they are supported.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: deny access to GIC regions
---
 xen/arch/arm/domain_build.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1e5ee0e..a4abf28 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1359,6 +1359,38 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
 #ifdef CONFIG_ACPI
 #define ACPI_DOM0_FDT_MIN_SIZE 4096
 
+static int acpi_iomem_deny_access(struct domain *d)
+{
+    acpi_status status;
+    struct acpi_table_spcr *spcr = NULL;
+    unsigned long gfn;
+    int rc;
+
+    /* Firstly permit full MMIO capabilities. */
+    rc = iomem_permit_access(d, 0UL, ~0UL);
+    if ( rc )
+        return rc;
+
+    /* TODO: Deny MMIO access for SMMU, GIC ITS */
+    status = acpi_get_table(ACPI_SIG_SPCR, 0,
+                            (struct acpi_table_header **)&spcr);
+
+    if ( ACPI_FAILURE(status) )
+    {
+        printk("Failed to get SPCR table\n");
+        return -EINVAL;
+    }
+
+    gfn = spcr->serial_port.address >> PAGE_SHIFT;
+    /* Deny MMIO access for UART */
+    rc = iomem_deny_access(d, gfn, gfn + 1);
+    if ( rc )
+        return rc;
+
+    /* Deny MMIO access for GIC regions */
+    return gic_iomem_deny_access(d);
+}
+
 static int acpi_permit_spi_access(struct domain *d)
 {
     int i, res;
@@ -1880,6 +1912,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
     if ( rc != 0 )
         return rc;
 
+    rc = acpi_iomem_deny_access(d);
+    if ( rc != 0 )
+        return rc;
+
     return 0;
 }
 #else
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (17 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0 Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 10:16   ` Jan Beulich
  2016-03-04 21:19   ` Konrad Rzeszutek Wilk
  2016-03-04  6:15 ` [PATCH v5 20/22] xen/acpi: Fix event-channel interrupt when booting with ACPI Shannon Zhao
                   ` (2 subsequent siblings)
  21 siblings, 2 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, Jan Beulich, zhaoshenglong

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

Add a new delivery type:
val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
bit 9 stands the interrupt polarity is active low(1) or high(0).

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: fix the statement
---
 xen/include/public/hvm/params.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 73d4718..2ea8d62 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -55,6 +55,16 @@
  * if this delivery method is available.
  */
 
+#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
+/*
+ * val[55:16] need to be zero.
+ * val[15:8] is flag of event-channel interrupt:
+ *  bit 8: interrupt is edge(1) or level(0) triggered
+ *  bit 9: interrupt is active low(1) or high(0)
+ * val[7:0] is PPI number used by event-channel.
+ * This is only used by ARM/ARM64.
+ */
+
 /*
  * These are not used by Xen. They are here for convenience of HVM-guest
  * xenbus implementations.
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 20/22] xen/acpi: Fix event-channel interrupt when booting with ACPI
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (18 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping Shannon Zhao
  2016-03-04  6:15 ` [PATCH v5 22/22] xen/arm64: Add ACPI support Shannon Zhao
  21 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, zhaoshenglong

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

Store the event-channel interrupt number and flag in HVM parameter
HVM_PARAM_CALLBACK_IRQ. Then Dom0 could get it through hypercall
HVMOP_get_param.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain_build.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a4abf28..5b1d583 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2008,6 +2008,7 @@ static void initrd_load(struct kernel_info *kinfo)
 static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
 {
     int res, node;
+    u64 val;
     gic_interrupt_t intr;
 
     /*
@@ -2023,6 +2024,15 @@ static void evtchn_fixup(struct domain *d, struct kernel_info *kinfo)
     printk("Allocating PPI %u for event channel interrupt\n",
            d->arch.evtchn_irq);
 
+    /* Set the value of domain param HVM_PARAM_CALLBACK_IRQ */
+    val = (u64)HVM_PARAM_CALLBACK_TYPE_EVENT << 56;
+    val |= (2 << 8); /* Active-low level-sensitive  */
+    val |= d->arch.evtchn_irq & 0xff;
+    d->arch.hvm_domain.params[HVM_PARAM_CALLBACK_IRQ] = val;
+
+    if ( !acpi_disabled )
+        return;
+
     /* Fix up "interrupts" in /hypervisor node */
     node = fdt_path_offset(kinfo->fdt, "/hypervisor");
     if ( node < 0 )
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (19 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 20/22] xen/acpi: Fix event-channel interrupt when booting with ACPI Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 10:29   ` Jan Beulich
  2016-03-04  6:15 ` [PATCH v5 22/22] xen/arm64: Add ACPI support Shannon Zhao
  21 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, Jan Beulich, zhaoshenglong

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

It needs to map platform or amba device mmio to Dom0 on ARM. But when
booting with ACPI, it can't get the mmio region in Xen due to lack of
AML interpreter to parse DSDT table. Therefore, let Dom0 call a
hypercall to map mmio region when it adds the devices.

Here we add a new map space like the XEN_DOMCTL_memory_mapping to map
mmio region for Dom0.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: fix coding style and commit message
---
 xen/arch/arm/mm.c           |  3 +++
 xen/arch/arm/p2m.c          | 23 +++++++++++++++++++++++
 xen/common/memory.c         | 16 ++++++++++++++++
 xen/include/asm-arm/p2m.h   |  5 +++++
 xen/include/public/memory.h |  1 +
 5 files changed, 48 insertions(+)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 81f9e2e..0aae6c5 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1138,6 +1138,9 @@ int xenmem_add_to_physmap_one(
         rcu_unlock_domain(od);
         break;
     }
+    case XENMAPSPACE_dev_mmio:
+        rc = map_dev_mmio_region(d, gpfn, 1, idx);
+        return rc;
 
     default:
         return -ENOSYS;
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index d206616..7264ed2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -7,6 +7,7 @@
 #include <xen/bitops.h>
 #include <xen/vm_event.h>
 #include <xen/mem_access.h>
+#include <xen/iocap.h>
 #include <public/vm_event.h>
 #include <asm/flushtlb.h>
 #include <asm/gic.h>
@@ -1270,6 +1271,28 @@ int unmap_mmio_regions(struct domain *d,
                              d->arch.p2m.default_access);
 }
 
+int map_dev_mmio_region(struct domain *d,
+                        unsigned long start_gfn,
+                        unsigned long nr,
+                        unsigned long mfn)
+{
+    int res;
+
+    if(!iomem_access_permitted(d, start_gfn, start_gfn + nr))
+        return 0;
+
+    res = map_mmio_regions(d, start_gfn, nr, mfn);
+    if ( res < 0 )
+    {
+        printk(XENLOG_ERR "Unable to map 0x%lx - 0x%lx in domain %d\n",
+               start_gfn << PAGE_SHIFT, (start_gfn + nr) << PAGE_SHIFT,
+               d->domain_id);
+        return res;
+    }
+
+    return 0;
+}
+
 int guest_physmap_add_entry(struct domain *d,
                             unsigned long gpfn,
                             unsigned long mfn,
diff --git a/xen/common/memory.c b/xen/common/memory.c
index ef57219..98db1cb 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -980,6 +980,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( d == NULL )
             return -ESRCH;
 
+        /*
+         * XENMAPSPACE_dev_mmio mapping is only supported for hardware Domain
+         * to map this kind of space to itself.
+         */
+        if ( (xatp.space == XENMAPSPACE_dev_mmio) &&
+             (!is_hardware_domain(current->domain) || (d != current->domain)) )
+            return -EACCES;
+
         rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d);
         if ( rc )
         {
@@ -1024,6 +1032,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( d == NULL )
             return -ESRCH;
 
+        /*
+         * XENMAPSPACE_dev_mmio mapping is only supported for hardware Domain
+         * to map this kind of space to itself.
+         */
+        if ( (xatpb.space == XENMAPSPACE_dev_mmio) &&
+             (!is_hardware_domain(current->domain) || (d != current->domain)) )
+            return -EACCES;
+
         rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d);
         if ( rc )
         {
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 17be6ad..5fc7ff3 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -154,6 +154,11 @@ int unmap_regions_rw(struct domain *d,
                     unsigned long nr_mfns,
                     unsigned long mfn);
 
+int map_dev_mmio_region(struct domain *d,
+                        unsigned long start_gfn,
+                        unsigned long nr,
+                        unsigned long mfn);
+
 int guest_physmap_add_entry(struct domain *d,
                             unsigned long gfn,
                             unsigned long mfn,
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index f69e92f..fe52ee1 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -220,6 +220,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
 #define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap only. */
 #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
                                     * XENMEM_add_to_physmap_batch only. */
+#define XENMAPSPACE_dev_mmio     5 /* device mmio region */
 /* ` } */
 
 /*
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* [PATCH v5 22/22] xen/arm64: Add ACPI support
  2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
                   ` (20 preceding siblings ...)
  2016-03-04  6:15 ` [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping Shannon Zhao
@ 2016-03-04  6:15 ` Shannon Zhao
  2016-03-04 10:33   ` Jan Beulich
  2016-03-04 11:45   ` Stefano Stabellini
  21 siblings, 2 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04  6:15 UTC (permalink / raw)
  To: xen-devel
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, Jan Beulich, zhaoshenglong

From: Naresh Bhat <naresh.bhat@linaro.org>

Add ACPI support on arm64 xen hypervisor. Enable EFI support on ARM.

Cc: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v5: make ACPI selectable option
---
 xen/arch/arm/Kconfig         |  9 +++++++++
 xen/common/efi/runtime.c     | 12 +++++++-----
 xen/include/asm-arm/config.h |  4 ++++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index cb99df5..25cec31 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -33,6 +33,15 @@ menu "Architecture Features"
 
 source "arch/Kconfig"
 
+config ACPI
+	bool "ACPI (Advanced Configuration and Power Interface) Support"
+	depends on ARM_64
+	default y
+	---help---
+
+	  Advanced Configuration and Power Interface (ACPI) support for Xen is
+	  an alternative to device tree on ARM64.
+
 # Select HAS_GICV3 if GICv3 is supported
 config HAS_GICV3
 	bool
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index ae87557..c256814 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -10,14 +10,16 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16);
 
 #ifndef COMPAT
 
-#ifdef CONFIG_ARM  /* Disabled until runtime services implemented */
-const bool_t efi_enabled = 0;
-#else
+/*
+ * Currently runtime services are not implemented on ARM. To boot Xen with ACPI,
+ * set efi_enabled to 1, so that Xen can get the ACPI root pointer from EFI.
+ */
+const bool_t efi_enabled = 1;
+
+#ifndef CONFIG_ARM
 # include <asm/i387.h>
 # include <asm/xstate.h>
 # include <public/platform.h>
-
-const bool_t efi_enabled = 1;
 #endif
 
 unsigned int __read_mostly efi_num_ct;
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 7ceb5c5..5fc9aa2 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -31,6 +31,10 @@
 
 #define CONFIG_ARM_L1_CACHE_SHIFT 7 /* XXX */
 
+#ifdef CONFIG_ACPI
+#define CONFIG_ACPI_BOOT 1
+#endif
+
 #define CONFIG_SMP 1
 
 #define CONFIG_IRQ_HAS_MULTIPLE_ACTION 1
-- 
2.0.4



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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04  6:15 ` [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables Shannon Zhao
@ 2016-03-04 10:09   ` Jan Beulich
  2016-03-04 10:55     ` Stefano Stabellini
  0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2016-03-04 10:09 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

>>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Estimate the memory required for loading acpi/efi tables in Dom0. Make
> the length of each table aligned with 64bit. Alloc the pages to store
> the new created EFI and ACPI tables and free these pages when
> destroying domain.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Since the pattern repeats I finally have to ask: Who is the author
of a patch with such a set of tag? You (From:) or Parth (first S-o-b)?

> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>      for( ; ; ); /* not reached */
>  }
>  
> +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
> +struct meminfo __init *get_acpi_meminfo(void)
> +{
> +    return &acpi_mem;
> +}
> +#endif

No such hackery in common code please, if at all avoidable. If ARM
maintainers are fine with this in their code, it could be put into
ARM's efi-boot.h.

> --- a/xen/common/efi/efi.h
> +++ b/xen/common/efi/efi.h
> @@ -39,3 +39,7 @@ extern UINT64 efi_boot_max_var_store_size, efi_boot_remain_var_store_size,
>  
>  unsigned long efi_rs_enter(void);
>  void efi_rs_leave(unsigned long);
> +
> +#ifdef CONFIG_ARM
> +struct meminfo *get_acpi_meminfo(void);
> +#endif

Similarly this should be put in an ARM specific header, which
efi-boot.h then should include (if it doesn't already).

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04  6:15 ` [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ Shannon Zhao
@ 2016-03-04 10:16   ` Jan Beulich
  2016-03-04 12:09     ` Stefano Stabellini
  2016-03-16 15:03     ` Julien Grall
  2016-03-04 21:19   ` Konrad Rzeszutek Wilk
  1 sibling, 2 replies; 63+ messages in thread
From: Jan Beulich @ 2016-03-04 10:16 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

>>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a new delivery type:
> val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
> To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
> bit 9 stands the interrupt polarity is active low(1) or high(0).
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

The set of Cc-s is too narrow - all REST maintainers should be copied.

> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -55,6 +55,16 @@
>   * if this delivery method is available.
>   */
>  
> +#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
> +/*
> + * val[55:16] need to be zero.
> + * val[15:8] is flag of event-channel interrupt:
> + *  bit 8: interrupt is edge(1) or level(0) triggered
> + *  bit 9: interrupt is active low(1) or high(0)
> + * val[7:0] is PPI number used by event-channel.
> + * This is only used by ARM/ARM64.
> + */

I think the name of the constant needs improvement. The low 8
bits make this extremely ARM specific, so perhaps
HVM_PARAM_CALLBACK_TYPE_PPI? Albeit - wouldn't the
vector and/or GSI ones be re-usable for this purpose? (I
don't know enough about ARM to be certain.)

And then I'm now also wondering about the description of bits
8 and 9 - event channels don't know of edge/level triggering
or high/low polarity, so it looks to me as if the comment is at
least misleading too.

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-04  6:15 ` [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping Shannon Zhao
@ 2016-03-04 10:29   ` Jan Beulich
  2016-03-04 11:00     ` Roger Pau Monné
  2016-03-16  9:48     ` Shannon Zhao
  0 siblings, 2 replies; 63+ messages in thread
From: Jan Beulich @ 2016-03-04 10:29 UTC (permalink / raw)
  To: Roger Pau Monne, Shannon Zhao, Boris Ostrovsky
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

>>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> It needs to map platform or amba device mmio to Dom0 on ARM. But when
> booting with ACPI, it can't get the mmio region in Xen due to lack of
> AML interpreter to parse DSDT table. Therefore, let Dom0 call a
> hypercall to map mmio region when it adds the devices.
> 
> Here we add a new map space like the XEN_DOMCTL_memory_mapping to map
> mmio region for Dom0.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Same remark regarding the Cc list.

> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1138,6 +1138,9 @@ int xenmem_add_to_physmap_one(
>          rcu_unlock_domain(od);
>          break;
>      }
> +    case XENMAPSPACE_dev_mmio:
> +        rc = map_dev_mmio_region(d, gpfn, 1, idx);

This being the only caller, ...

> +int map_dev_mmio_region(struct domain *d,
> +                        unsigned long start_gfn,
> +                        unsigned long nr,
> +                        unsigned long mfn)
> +{

... what's the "nr" parameter good for? Or alternatively - wouldn't
you want to make it possible to have larger areas mapped by large
pages?

> +    int res;
> +
> +    if(!iomem_access_permitted(d, start_gfn, start_gfn + nr))
> +        return 0;

This would seem to belong into common code

Also - coding style.

> +    res = map_mmio_regions(d, start_gfn, nr, mfn);
> +    if ( res < 0 )
> +    {
> +        printk(XENLOG_ERR "Unable to map 0x%lx - 0x%lx in domain %d\n",

%#lx

> +               start_gfn << PAGE_SHIFT, (start_gfn + nr) << PAGE_SHIFT,

I see no reason for the shifts.

> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -980,6 +980,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( d == NULL )
>              return -ESRCH;
>  
> +        /*
> +         * XENMAPSPACE_dev_mmio mapping is only supported for hardware Domain
> +         * to map this kind of space to itself.
> +         */
> +        if ( (xatp.space == XENMAPSPACE_dev_mmio) &&
> +             (!is_hardware_domain(current->domain) || (d != current->domain)) )

Readability would benefit if you used "d" twice and "current->domain"
just once, preferable after swapping the two sides of the ||.

Overall I wonder whether this wouldn't help PVH on x86 too,
where we currently do some hackery to (not even completely)
map MMIO into Dom0's p2m. In such a case perhaps
map_dev_mmio_regions() should become a general per-arch
function right away (declared in a common header and stubbed
out in x86 code for now). Boris, Roger?

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 22/22] xen/arm64: Add ACPI support
  2016-03-04  6:15 ` [PATCH v5 22/22] xen/arm64: Add ACPI support Shannon Zhao
@ 2016-03-04 10:33   ` Jan Beulich
  2016-03-04 11:45     ` Stefano Stabellini
  2016-03-04 11:45   ` Stefano Stabellini
  1 sibling, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2016-03-04 10:33 UTC (permalink / raw)
  To: stefano.stabellini, Shannon Zhao; +Cc: hangaohuai, shannon.zhao, xen-devel

>>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> From: Naresh Bhat <naresh.bhat@linaro.org>
> 
> Add ACPI support on arm64 xen hypervisor. Enable EFI support on ARM.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: make ACPI selectable option
> ---
>  xen/arch/arm/Kconfig         |  9 +++++++++
>  xen/common/efi/runtime.c     | 12 +++++++-----

For the change to this file:
Acked-by: Jan Beulich <jbeulich@suse.com>
However ...

> +++ b/xen/arch/arm/Kconfig
> @@ -33,6 +33,15 @@ menu "Architecture Features"
>  
>  source "arch/Kconfig"
>  
> +config ACPI
> +	bool "ACPI (Advanced Configuration and Power Interface) Support"
> +	depends on ARM_64
> +	default y

... I recall Stefano having asked for this, but - are both of you sure?
Have you seen the earlier discussion about the introduction of new
user selectable config options? I.e. shouldn't the prompt at least
become conditional on EXPERT = "y", just like done elsewhere?

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 09/22] arm/p2m: Add helper functions to map memory regions
  2016-03-04  6:15 ` [PATCH v5 09/22] arm/p2m: Add helper functions to map memory regions Shannon Zhao
@ 2016-03-04 10:51   ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 10:51 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Parth Dixit <parth.dixit@linaro.org>
> 
> Create a helper function for mapping with cached attributes and
> read-write range.
> 
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: use p2m_access_rw and rename to map_regions_rw

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  xen/arch/arm/p2m.c        | 26 ++++++++++++++++++++++++++
>  xen/include/asm-arm/p2m.h | 10 ++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index a2a9c4b..d206616 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1218,6 +1218,32 @@ int p2m_populate_ram(struct domain *d,
>                               d->arch.p2m.default_access);
>  }
>  
> +int map_regions_rw(struct domain *d,
> +                     unsigned long start_gfn,
> +                     unsigned long nr,
> +                     unsigned long mfn)
> +{
> +    return apply_p2m_changes(d, INSERT,
> +                             pfn_to_paddr(start_gfn),
> +                             pfn_to_paddr(start_gfn + nr),
> +                             pfn_to_paddr(mfn),
> +                             MATTR_MEM, 0, p2m_mmio_direct,
> +                             p2m_access_rw);
> +}
> +
> +int unmap_regions_rw(struct domain *d,
> +                       unsigned long start_gfn,
> +                       unsigned long nr,
> +                       unsigned long mfn)
> +{
> +    return apply_p2m_changes(d, REMOVE,
> +                             pfn_to_paddr(start_gfn),
> +                             pfn_to_paddr(start_gfn + nr),
> +                             pfn_to_paddr(mfn),
> +                             MATTR_MEM, 0, p2m_invalid,
> +                             p2m_access_rw);
> +}
> +
>  int map_mmio_regions(struct domain *d,
>                       unsigned long start_gfn,
>                       unsigned long nr,
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 433952a..17be6ad 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -144,6 +144,16 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
>  /* Setup p2m RAM mapping for domain d from start-end. */
>  int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
>  
> +int map_regions_rw(struct domain *d,
> +                    unsigned long start_gfn,
> +                    unsigned long nr_mfns,
> +                    unsigned long mfn);
> +
> +int unmap_regions_rw(struct domain *d,
> +                    unsigned long start_gfn,
> +                    unsigned long nr_mfns,
> +                    unsigned long mfn);
> +
>  int guest_physmap_add_entry(struct domain *d,
>                              unsigned long gfn,
>                              unsigned long mfn,
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04 10:09   ` Jan Beulich
@ 2016-03-04 10:55     ` Stefano Stabellini
  2016-03-04 15:03       ` Shannon Zhao
  0 siblings, 1 reply; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 10:55 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, xen-devel, stefano.stabellini, shannon.zhao, Shannon Zhao

On Fri, 4 Mar 2016, Jan Beulich wrote:
> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > Estimate the memory required for loading acpi/efi tables in Dom0. Make
> > the length of each table aligned with 64bit. Alloc the pages to store
> > the new created EFI and ACPI tables and free these pages when
> > destroying domain.
> > 
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Since the pattern repeats I finally have to ask: Who is the author
> of a patch with such a set of tag? You (From:) or Parth (first S-o-b)?
> 
> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
> >      for( ; ; ); /* not reached */
> >  }
> >  
> > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
> > +struct meminfo __init *get_acpi_meminfo(void)
> > +{
> > +    return &acpi_mem;
> > +}
> > +#endif
> 
> No such hackery in common code please, if at all avoidable. If ARM
> maintainers are fine with this in their code, it could be put into
> ARM's efi-boot.h.

I am OK with that. If you move it under arch/arm, then drop the ifdef
CONFIG_ARM.


> > --- a/xen/common/efi/efi.h
> > +++ b/xen/common/efi/efi.h
> > @@ -39,3 +39,7 @@ extern UINT64 efi_boot_max_var_store_size, efi_boot_remain_var_store_size,
> >  
> >  unsigned long efi_rs_enter(void);
> >  void efi_rs_leave(unsigned long);
> > +
> > +#ifdef CONFIG_ARM
> > +struct meminfo *get_acpi_meminfo(void);
> > +#endif
> 
> Similarly this should be put in an ARM specific header, which
> efi-boot.h then should include (if it doesn't already).

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset
  2016-03-04  6:15 ` [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset Shannon Zhao
@ 2016-03-04 10:59   ` Stefano Stabellini
  2016-03-04 15:12     ` Shannon Zhao
  0 siblings, 1 reply; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 10:59 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> These tables are aligned with 64bit.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: make the return value type and variable type consistent
> ---
>  xen/arch/arm/acpi/lib.c    | 15 +++++++++++++++
>  xen/include/asm-arm/acpi.h |  6 ++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> index db5c4d8..79f7edd 100644
> --- a/xen/arch/arm/acpi/lib.c
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -60,3 +60,18 @@ bool_t __init acpi_psci_hvc_present(void)
>  {
>      return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
>  }
> +
> +paddr_t __init acpi_get_table_offset(struct membank tbl_add[],
> +                                     EFI_MEM_RES index)
> +{
> +    int i;
> +    paddr_t offset = 0;
> +
> +    for ( i = 0; i < index; i++ )
> +    {
> +        /* Aligned with 64bit (8 bytes) */
> +        offset += ROUNDUP(tbl_add[i].size, 8);
> +    }
> +
> +    return offset;
> +}
> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
> index 7f59761..6db3711 100644
> --- a/xen/include/asm-arm/acpi.h
> +++ b/xen/include/asm-arm/acpi.h
> @@ -25,6 +25,7 @@
>  
>  #include <xen/init.h>
>  #include <asm/page.h>
> +#include <asm/setup.h>
>  
>  #define COMPILER_DEPENDENT_INT64   long long
>  #define COMPILER_DEPENDENT_UINT64  unsigned long long
> @@ -58,10 +59,15 @@ static inline void enable_acpi(void)
>  {
>      acpi_disabled = 0;
>  }
> +
> +paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index);
>  #else
>  #define acpi_disabled (1)
>  #define disable_acpi()
>  #define enable_acpi()
> +paddr_t inline acpi_get_table_offset(struct membank tbl_add[],
> +                                     EFI_MEM_RES index)
> +{ return 0; }

Why did you move the declaration of acpi_get_table_offset to within the
ifdef?  It was just above the ifdef in the previous version of the
patch.


>  #endif
>  
>  #endif /*_ASM_ARM_ACPI_H*/
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-04 10:29   ` Jan Beulich
@ 2016-03-04 11:00     ` Roger Pau Monné
  2016-03-04 11:11       ` Jan Beulich
  2016-03-16  9:48     ` Shannon Zhao
  1 sibling, 1 reply; 63+ messages in thread
From: Roger Pau Monné @ 2016-03-04 11:00 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, xen-devel, stefano.stabellini, shannon.zhao,
	Shannon Zhao, Boris Ostrovsky, Roger Pau Monne

On Fri, 4 Mar 2016, Jan Beulich wrote:
> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> Overall I wonder whether this wouldn't help PVH on x86 too,
> where we currently do some hackery to (not even completely)
> map MMIO into Dom0's p2m. In such a case perhaps
> map_dev_mmio_regions() should become a general per-arch
> function right away (declared in a common header and stubbed
> out in x86 code for now). Boris, Roger?

I though that we agreed that Xen will take care of doing all the MMIO 
mappings for HVMlite/PVHv2 guests:

http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01609.html

My first idea was to use the MMIO device mapping hypercall (just like 
ARM) on the hardware domain, but I think we can get away without it on x86.

Roger.

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 11/22] arm/acpi: Prepare EFI system table for Dom0
  2016-03-04  6:15 ` [PATCH v5 11/22] arm/acpi: Prepare EFI system table " Shannon Zhao
@ 2016-03-04 11:02   ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:02 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Prepare EFI system table for Dom0 to describe the information of UEFI.
> 
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


> v5: move to efi-dom0.c
> ---
>  xen/arch/arm/domain_build.c |  2 ++
>  xen/arch/arm/efi/efi-dom0.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/setup.h |  3 +++
>  3 files changed, 49 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c71976c..613551c 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1686,6 +1686,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>          return rc;
>  
>      acpi_map_other_tables(d);
> +    acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
> +                                 tbl_add);
>  
>      return 0;
>  }
> diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
> index d54c38f..36a1283 100644
> --- a/xen/arch/arm/efi/efi-dom0.c
> +++ b/xen/arch/arm/efi/efi-dom0.c
> @@ -46,3 +46,47 @@ size_t __init estimate_efi_size(int mem_nr_banks)
>  
>      return size;
>  }
> +
> +#include "../../../common/decompress.h"
> +#define XZ_EXTERN STATIC
> +#include "../../../common/xz/crc32.c"
> +
> +void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
> +                                         struct membank tbl_add[])
> +{
> +    u64 table_addr, table_size, offset = 0;
> +    u8 *base_ptr;
> +    EFI_CONFIGURATION_TABLE *efi_conf_tbl;
> +    EFI_SYSTEM_TABLE *efi_sys_tbl;
> +
> +    table_addr = paddr + acpi_get_table_offset(tbl_add, TBL_EFIT);
> +    table_size = sizeof(EFI_SYSTEM_TABLE) + sizeof(EFI_CONFIGURATION_TABLE)
> +                 + sizeof(xen_efi_fw_vendor);
> +    base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_EFIT);
> +    efi_sys_tbl = (EFI_SYSTEM_TABLE *)base_ptr;
> +
> +    efi_sys_tbl->Hdr.Signature = EFI_SYSTEM_TABLE_SIGNATURE;
> +    /* Specify the revision as 2.5 */
> +    efi_sys_tbl->Hdr.Revision = (2 << 16 | 50);
> +    efi_sys_tbl->Hdr.HeaderSize = table_size;
> +
> +    efi_sys_tbl->FirmwareRevision = 1;
> +    efi_sys_tbl->NumberOfTableEntries = 1;
> +    offset += sizeof(EFI_SYSTEM_TABLE);
> +    memcpy((CHAR16 *)(base_ptr + offset), xen_efi_fw_vendor,
> +           sizeof(xen_efi_fw_vendor));
> +    efi_sys_tbl->FirmwareVendor = (CHAR16 *)(table_addr + offset);
> +
> +    offset += sizeof(xen_efi_fw_vendor);
> +    efi_conf_tbl = (EFI_CONFIGURATION_TABLE *)(base_ptr + offset);
> +    efi_conf_tbl->VendorGuid = (EFI_GUID)ACPI_20_TABLE_GUID;
> +    efi_conf_tbl->VendorTable = (VOID *)tbl_add[TBL_RSDP].start;
> +    efi_sys_tbl->ConfigurationTable = (EFI_CONFIGURATION_TABLE *)(table_addr
> +                                                                  + offset);
> +    xz_crc32_init();
> +    efi_sys_tbl->Hdr.CRC32 = xz_crc32((uint8_t *)efi_sys_tbl,
> +                                      efi_sys_tbl->Hdr.HeaderSize, 0);
> +
> +    tbl_add[TBL_EFIT].start = table_addr;
> +    tbl_add[TBL_EFIT].size = table_size;
> +}
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 7f233a1..e423b15 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -53,6 +53,9 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
>  
>  size_t estimate_efi_size(int mem_nr_banks);
>  
> +void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
> +                                  struct membank tbl_add[]);
> +
>  int construct_dom0(struct domain *d);
>  
>  void discard_initial_modules(void);
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-04 11:00     ` Roger Pau Monné
@ 2016-03-04 11:11       ` Jan Beulich
  2016-03-04 11:37         ` Stefano Stabellini
  0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2016-03-04 11:11 UTC (permalink / raw)
  To: Roger Pau Monne, stefano.stabellini, Shannon Zhao
  Cc: Boris Ostrovsky, hangaohuai, shannon.zhao, xen-devel

>>> On 04.03.16 at 12:00, <roger.pau@citrix.com> wrote:
> On Fri, 4 Mar 2016, Jan Beulich wrote:
>> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
>> Overall I wonder whether this wouldn't help PVH on x86 too,
>> where we currently do some hackery to (not even completely)
>> map MMIO into Dom0's p2m. In such a case perhaps
>> map_dev_mmio_regions() should become a general per-arch
>> function right away (declared in a common header and stubbed
>> out in x86 code for now). Boris, Roger?
> 
> I though that we agreed that Xen will take care of doing all the MMIO 
> mappings for HVMlite/PVHv2 guests:
> 
> http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01609.html 
> 
> My first idea was to use the MMIO device mapping hypercall (just like 
> ARM) on the hardware domain, but I think we can get away without it on x86.

Oh, you're right (albeit that doesn't cover MMIO regions not
represented by BARs, which will need taking care of). Question
back to the ARM folks then: Would such a model work for you
too? (I'd really like to avoid having two different models, if we
can avoid it.)

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0
  2016-03-04  6:15 ` [PATCH v5 12/22] arm/acpi: Prepare EFI memory descriptor " Shannon Zhao
@ 2016-03-04 11:13   ` Stefano Stabellini
  2016-03-16  8:59     ` Shannon Zhao
  0 siblings, 1 reply; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:13 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Create a few EFI memory descriptors to tell Dom0 the RAM region
> information, ACPI table regions and EFI tables reserved resions.
> 
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: move to efi-dom0.c
> ---
>  xen/arch/arm/domain_build.c |  2 ++
>  xen/arch/arm/efi/efi-dom0.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/setup.h |  5 +++++
>  3 files changed, 54 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 613551c..008fc76 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>      acpi_map_other_tables(d);
>      acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
>                                   tbl_add);
> +    acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
> +                               d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
>  
>      return 0;
>  }
> diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
> index 36a1283..0ff6309 100644
> --- a/xen/arch/arm/efi/efi-dom0.c
> +++ b/xen/arch/arm/efi/efi-dom0.c
> @@ -22,6 +22,7 @@
>   */
>  
>  #include "efi.h"
> +#include <xen/pfn.h>
>  #include <asm/setup.h>
>  #include <asm/acpi.h>
>  
> @@ -90,3 +91,49 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
>      tbl_add[TBL_EFIT].start = table_addr;
>      tbl_add[TBL_EFIT].size = table_size;
>  }
> +
> +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
> +                                       void *efi_acpi_table,
> +                                       const struct meminfo *mem,
> +                                       struct membank tbl_add[])
> +{
> +    EFI_MEMORY_DESCRIPTOR *memory_map;
> +    struct meminfo *acpi_mem;
> +    int acpi_mem_nr_banks = 0;
> +    unsigned int i, offset;
> +    u8 *base_ptr;
> +
> +    base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP);
> +    memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr);
> +
> +    offset = 0;
> +    for( i = 0; i < mem->nr_banks; i++, offset++ )
> +    {
> +        memory_map[offset].Type = EfiConventionalMemory;
> +        memory_map[offset].PhysicalStart = mem->bank[i].start;
> +        memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size);
> +        memory_map[offset].Attribute = EFI_MEMORY_WB;
> +    }
> +
> +    if ( !acpi_disabled )
> +    {

Isn't this check redundant, givem that the function is name is
acpi_create_efi_mmap_table and is called by prepare_acpi?


> +        acpi_mem = get_acpi_meminfo();

Would it be possible to call get_acpi_meminfo from prepare_acpi and pass
acpi_mem to this function?


> +        acpi_mem_nr_banks = acpi_mem->nr_banks;
> +        for( i = 0; i < acpi_mem_nr_banks; i++, offset++ )
> +        {
> +            memory_map[offset].Type = EfiACPIReclaimMemory;
> +            memory_map[offset].PhysicalStart = acpi_mem->bank[i].start;
> +            memory_map[offset].NumberOfPages = PFN_UP(acpi_mem->bank[i].size);
> +            memory_map[offset].Attribute = EFI_MEMORY_WB;
> +        }
> +    }
> +
> +    memory_map[offset].Type = EfiACPIReclaimMemory;
> +    memory_map[offset].PhysicalStart = paddr;
> +    memory_map[offset].NumberOfPages = PFN_UP(size);
> +    memory_map[offset].Attribute = EFI_MEMORY_WB;
> +
> +    tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
> +    tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
> +                             * (mem->nr_banks + acpi_mem_nr_banks + 1);
> +}
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index e423b15..b2899f2 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -56,6 +56,11 @@ size_t estimate_efi_size(int mem_nr_banks);
>  void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
>                                    struct membank tbl_add[]);
>  
> +void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
> +                                void *efi_acpi_table,
> +                                const struct meminfo *mem,
> +                                struct membank tbl_add[]);
> +
>  int construct_dom0(struct domain *d);
>  
>  void discard_initial_modules(void);
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 14/22] arm/acpi: Create min DT stub for Dom0
  2016-03-04  6:15 ` [PATCH v5 14/22] arm/acpi: Create min DT stub for Dom0 Shannon Zhao
@ 2016-03-04 11:17   ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:17 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Create a DT for Dom0 for ACPI-case only. DT contains minimal required
> informations such as Dom0 bootargs, initrd, efi description table and
> address of uefi memory table.
> 
> Also port the document of this device tree bindings from Linux.
> 
> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: change the file name

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  docs/misc/arm/device-tree/uefi.txt |  58 +++++++++++++++
>  xen/arch/arm/domain_build.c        | 143 +++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/efi/efi-dom0.c        |  48 +++++++++++++
>  xen/include/asm-arm/setup.h        |   2 +
>  4 files changed, 251 insertions(+)
>  create mode 100644 docs/misc/arm/device-tree/uefi.txt
> 
> diff --git a/docs/misc/arm/device-tree/uefi.txt b/docs/misc/arm/device-tree/uefi.txt
> new file mode 100644
> index 0000000..41a8be0
> --- /dev/null
> +++ b/docs/misc/arm/device-tree/uefi.txt
> @@ -0,0 +1,58 @@
> +* Xen hypervisor device tree bindings
> +
> +Xen ARM virtual platforms shall have a top-level "hypervisor" node with
> +the following properties:
> +
> +- compatible:
> +	compatible = "xen,xen-<version>", "xen,xen";
> +  where <version> is the version of the Xen ABI of the platform.
> +
> +- reg: specifies the base physical address and size of a region in
> +  memory where the grant table should be mapped to, using an
> +  HYPERVISOR_memory_op hypercall. The memory region is large enough to map
> +  the whole grant table (it is larger or equal to gnttab_max_grant_frames()).
> +
> +- interrupts: the interrupt used by Xen to inject event notifications.
> +  A GIC node is also required.
> +
> +To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
> +under /hypervisor with following parameters:
> +
> +________________________________________________________________________________
> +Name                      | Size   | Description
> +================================================================================
> +xen,uefi-system-table     | 64-bit | Guest physical address of the UEFI System
> +                          |        | Table.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-start       | 64-bit | Guest physical address of the UEFI memory
> +                          |        | map.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-size        | 32-bit | Size in bytes of the UEFI memory map
> +                          |        | pointed to in previous entry.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
> +                          |        | memory map.
> +--------------------------------------------------------------------------------
> +xen,uefi-mmap-desc-ver    | 32-bit | Version of the mmap descriptor format.
> +--------------------------------------------------------------------------------
> +
> +Example (assuming #address-cells = <2> and #size-cells = <2>):
> +
> +hypervisor {
> +	compatible = "xen,xen-4.3", "xen,xen";
> +	reg = <0 0xb0000000 0 0x20000>;
> +	interrupts = <1 15 0xf08>;
> +	uefi {
> +		xen,uefi-system-table = <0xXXXXXXXX>;
> +		xen,uefi-mmap-start = <0xXXXXXXXX>;
> +		xen,uefi-mmap-size = <0xXXXXXXXX>;
> +		xen,uefi-mmap-desc-size = <0xXXXXXXXX>;
> +		xen,uefi-mmap-desc-ver = <0xXXXXXXXX>;
> +        };
> +};
> +
> +The format and meaning of the "xen,uefi-*" parameters are similar to those in
> +Documentation/arm/uefi.txt in Linux, which are provided by the regular Linux
> +UEFI stub. However they differ because they are provided by the Xen hypervisor,
> +together with a set of UEFI runtime services implemented via hypercalls, see
> +xen/include/public/platform.h.
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index e036887..6726e45 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1357,6 +1357,145 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>  }
>  
>  #ifdef CONFIG_ACPI
> +#define ACPI_DOM0_FDT_MIN_SIZE 4096
> +
> +static int make_chosen_node(const struct kernel_info *kinfo,
> +                            struct membank tbl_add[])
> +{
> +    int res;
> +    const char *bootargs = NULL;
> +    const struct bootmodule *mod = kinfo->kernel_bootmodule;
> +    void *fdt = kinfo->fdt;
> +
> +    DPRINT("Create chosen node\n");
> +    res = fdt_begin_node(fdt, "chosen");
> +    if ( res )
> +        return res;
> +
> +    if ( mod && mod->cmdline[0] )
> +    {
> +        bootargs = &mod->cmdline[0];
> +        res = fdt_property(fdt, "bootargs", bootargs, strlen(bootargs) + 1);
> +        if ( res )
> +           return res;
> +    }
> +
> +    /*
> +     * If the bootloader provides an initrd, we must create a placeholder
> +     * for the initrd properties. The values will be replaced later.
> +     */
> +    if ( mod && mod->size )
> +    {
> +        u64 a = 0;
> +        res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a));
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a));
> +        if ( res )
> +            return res;
> +    }
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}
> +
> +static int acpi_make_hypervisor_node(const struct kernel_info *kinfo,
> +                                     struct membank tbl_add[])
> +{
> +    const char compat[] =
> +        "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
> +        "xen,xen";
> +    int res;
> +    /* Convenience alias */
> +    void *fdt = kinfo->fdt;
> +
> +    DPRINT("Create hypervisor node\n");
> +
> +    /* See linux Documentation/devicetree/bindings/arm/xen.txt */
> +    res = fdt_begin_node(fdt, "hypervisor");
> +    if ( res )
> +        return res;
> +
> +    /* Cannot use fdt_property_string due to embedded nulls */
> +    res = fdt_property(fdt, "compatible", compat, sizeof(compat));
> +    if ( res )
> +        return res;
> +
> +    res = arm_acpi_make_efi_nodes(fdt, tbl_add);
> +    if ( res )
> +        return res;
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}
> +
> +/*
> + * Prepare a minimal DTB for Dom0 which contains bootargs, initrd, memory
> + * information, EFI table.
> + */
> +static int create_acpi_dtb(struct kernel_info *kinfo, struct membank tbl_add[])
> +{
> +    int new_size;
> +    int ret;
> +
> +    DPRINT("Prepare a min DTB for DOM0\n");
> +
> +    /* Allocate min size for DT */
> +    new_size = ACPI_DOM0_FDT_MIN_SIZE;
> +    kinfo->fdt = xmalloc_bytes(new_size);
> +
> +    if ( kinfo->fdt == NULL )
> +        return -ENOMEM;
> +
> +    /* Create a new empty DT for DOM0 */
> +    ret = fdt_create(kinfo->fdt, new_size);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    ret = fdt_finish_reservemap(kinfo->fdt);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    ret = fdt_begin_node(kinfo->fdt, "/");
> +    if ( ret < 0 )
> +        goto err;
> +
> +    ret = fdt_property_cell(kinfo->fdt, "#address-cells", 2);
> +    if ( ret )
> +        return ret;
> +
> +    ret = fdt_property_cell(kinfo->fdt, "#size-cells", 1);
> +    if ( ret )
> +        return ret;
> +
> +    /* Create a chosen node for DOM0 */
> +    ret = make_chosen_node(kinfo, tbl_add);
> +    if ( ret )
> +        goto err;
> +
> +    ret = acpi_make_hypervisor_node(kinfo, tbl_add);
> +    if ( ret )
> +        goto err;
> +
> +    ret = fdt_end_node(kinfo->fdt);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    ret = fdt_finish(kinfo->fdt);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    return 0;
> +
> +  err:
> +    printk("Device tree generation failed (%d).\n", ret);
> +    xfree(kinfo->fdt);
> +    return -EINVAL;
> +}
> +
>  static void acpi_map_other_tables(struct domain *d)
>  {
>      int i;
> @@ -1706,6 +1845,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>          return rc;
>      }
>  
> +    rc = create_acpi_dtb(kinfo, tbl_add);
> +    if ( rc != 0 )
> +        return rc;
> +
>      return 0;
>  }
>  #else
> diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
> index 0ff6309..3a7b601 100644
> --- a/xen/arch/arm/efi/efi-dom0.c
> +++ b/xen/arch/arm/efi/efi-dom0.c
> @@ -23,6 +23,7 @@
>  
>  #include "efi.h"
>  #include <xen/pfn.h>
> +#include <xen/libfdt/libfdt.h>
>  #include <asm/setup.h>
>  #include <asm/acpi.h>
>  
> @@ -137,3 +138,50 @@ void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
>      tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
>                               * (mem->nr_banks + acpi_mem_nr_banks + 1);
>  }
> +
> +/* Create place holder for efi values. */
> +int __init arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[])
> +{
> +    u64 fdt_val64;
> +    u32 fdt_val32;
> +    int desc_ver = 1;
> +    int res;
> +
> +    res = fdt_begin_node(fdt, "uefi");
> +    if ( res )
> +        return res;
> +
> +    fdt_val64 = cpu_to_fdt64(tbl_add[TBL_EFIT].start);
> +    res = fdt_property(fdt, "xen,uefi-system-table",
> +                       &fdt_val64, sizeof(fdt_val64));
> +    if ( res )
> +        return res;
> +
> +    fdt_val64 = cpu_to_fdt64(tbl_add[TBL_MMAP].start);
> +    res = fdt_property(fdt, "xen,uefi-mmap-start",
> +                       &fdt_val64,  sizeof(fdt_val64));
> +    if ( res )
> +        return res;
> +
> +    fdt_val32 = cpu_to_fdt32(tbl_add[TBL_MMAP].size);
> +    res = fdt_property(fdt, "xen,uefi-mmap-size",
> +                       &fdt_val32,  sizeof(fdt_val32));
> +    if ( res )
> +        return res;
> +
> +    fdt_val32 = cpu_to_fdt32(sizeof(EFI_MEMORY_DESCRIPTOR));
> +    res = fdt_property(fdt, "xen,uefi-mmap-desc-size",
> +                         &fdt_val32, sizeof(fdt_val32));
> +    if ( res )
> +        return res;
> +
> +    fdt_val32 = cpu_to_fdt32(desc_ver);
> +    res = fdt_property(fdt, "xen,uefi-mmap-desc-ver",
> +                         &fdt_val32, sizeof(fdt_val32));
> +    if ( res )
> +        return res;
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index b2899f2..05f0210 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -61,6 +61,8 @@ void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
>                                  const struct meminfo *mem,
>                                  struct membank tbl_add[]);
>  
> +int arm_acpi_make_efi_nodes(void *fdt, struct membank tbl_add[]);
> +
>  int construct_dom0(struct domain *d);
>  
>  void discard_initial_modules(void);
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically
  2016-03-04  6:15 ` [PATCH v5 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically Shannon Zhao
@ 2016-03-04 11:26   ` Stefano Stabellini
  2016-03-04 15:16     ` Shannon Zhao
  0 siblings, 1 reply; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:26 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Interrupt information is described in DSDT and is not available at the
> time of booting. Check if the interrupt is permitted to access and set
> the interrupt type, route it to guest dynamically only for SPI
> and Dom0.
> 
> Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: add a small function to get the irq type from ICFG
> ---
>  xen/arch/arm/vgic.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index ee35683..52378a3 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -25,6 +25,8 @@
>  #include <xen/irq.h>
>  #include <xen/sched.h>
>  #include <xen/perfc.h>
> +#include <xen/iocap.h>
> +#include <xen/acpi.h>
>  
>  #include <asm/current.h>
>  
> @@ -334,6 +336,21 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
>      }
>  }
>  
> +#ifdef CONFIG_ACPI
> +#define VGIC_ICFG_MASK(intr) ( 1 << ( ( 2 * ( intr % 16 ) ) + 1 ) )

                  remove spaces: ^      ^                   ^       ^

> +static unsigned int get_the_irq_type(struct vcpu *v, int n, int index)

static inline


> +{
> +    struct vgic_irq_rank *vr = vgic_get_rank(v, n);
> +    uint32_t tr = vr->icfg[index >> 4];
> +
> +    if ( tr & VGIC_ICFG_MASK(index) )
> +        return IRQ_TYPE_EDGE_BOTH;
> +    else
> +        return IRQ_TYPE_LEVEL_MASK;
> +}
> +#endif
> +
>  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>  {
>      const unsigned long mask = r;
> @@ -342,9 +359,30 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>      unsigned long flags;
>      int i = 0;
>      struct vcpu *v_target;
> +#ifdef CONFIG_ACPI
> +    struct domain *d = v->domain;
> +    int ret;
> +#endif
>  
>      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>          irq = i + (32 * n);
> +#ifdef CONFIG_ACPI
> +        /* Set the irq type and route it to guest only for SPI and Dom0 */
> +        if( irq_access_permitted(d, irq) && is_hardware_domain(d) &&
> +            ( irq >= 32 ) && ( !acpi_disabled ) )

Is there a reason why we need to have this code inside an ifdef? It
looks like we can remove all ifdefs from this patch.


> +        {
> +            ret = irq_set_spi_type(irq, get_the_irq_type(v, n, i));
> +            if ( ret )
> +                printk(XENLOG_WARNING "The irq type is not correct\n");
> +
> +            vgic_reserve_virq(d, irq);
> +
> +            ret = route_irq_to_guest(d, irq, irq, NULL);
> +            if ( ret )
> +                printk(XENLOG_ERR "Unable to route IRQ %u to domain %u\n",
> +                       irq, d->domain_id);
> +        }
> +#endif
>          v_target = __vgic_get_target_vcpu(v, irq);
>          p = irq_to_pending(v_target, irq);
>          set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-04 11:11       ` Jan Beulich
@ 2016-03-04 11:37         ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:37 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, xen-devel, stefano.stabellini, shannon.zhao,
	Shannon Zhao, Boris Ostrovsky, Roger Pau Monne

On Fri, 4 Mar 2016, Jan Beulich wrote:
> >>> On 04.03.16 at 12:00, <roger.pau@citrix.com> wrote:
> > On Fri, 4 Mar 2016, Jan Beulich wrote:
> >> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> >> Overall I wonder whether this wouldn't help PVH on x86 too,
> >> where we currently do some hackery to (not even completely)
> >> map MMIO into Dom0's p2m. In such a case perhaps
> >> map_dev_mmio_regions() should become a general per-arch
> >> function right away (declared in a common header and stubbed
> >> out in x86 code for now). Boris, Roger?
> > 
> > I though that we agreed that Xen will take care of doing all the MMIO 
> > mappings for HVMlite/PVHv2 guests:
> > 
> > http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01609.html 
> > 
> > My first idea was to use the MMIO device mapping hypercall (just like 
> > ARM) on the hardware domain, but I think we can get away without it on x86.
> 
> Oh, you're right (albeit that doesn't cover MMIO regions not
> represented by BARs, which will need taking care of).

This.

On ARM there can MMIO regions of non-PCI devices which need to be mapped
and can only be discovered via DSDT.


> Question
> back to the ARM folks then: Would such a model work for you
> too? (I'd really like to avoid having two different models, if we
> can avoid it.)

It doesn't cover non-PCI devices. But once you extend your model to deal
with non-PCI devices and MMIO regions not represented by BARs, then
potentually yes.

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 22/22] xen/arm64: Add ACPI support
  2016-03-04 10:33   ` Jan Beulich
@ 2016-03-04 11:45     ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:45 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, xen-devel, stefano.stabellini, shannon.zhao, Shannon Zhao

On Fri, 4 Mar 2016, Jan Beulich wrote:
> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> > From: Naresh Bhat <naresh.bhat@linaro.org>
> > 
> > Add ACPI support on arm64 xen hypervisor. Enable EFI support on ARM.
> > 
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> > v5: make ACPI selectable option
> > ---
> >  xen/arch/arm/Kconfig         |  9 +++++++++
> >  xen/common/efi/runtime.c     | 12 +++++++-----
> 
> For the change to this file:
> Acked-by: Jan Beulich <jbeulich@suse.com>
> However ...
> 
> > +++ b/xen/arch/arm/Kconfig
> > @@ -33,6 +33,15 @@ menu "Architecture Features"
> >  
> >  source "arch/Kconfig"
> >  
> > +config ACPI
> > +	bool "ACPI (Advanced Configuration and Power Interface) Support"
> > +	depends on ARM_64
> > +	default y
> 
> ... I recall Stefano having asked for this, but - are both of you sure?
> Have you seen the earlier discussion about the introduction of new
> user selectable config options? I.e. shouldn't the prompt at least
> become conditional on EXPERT = "y", just like done elsewhere?
 
ACPI is big chunk of code which people might want to reasonably disable
when booting on anything but servers. Think of all the small dev boards
(Cubieboard), or embedded use cases (set top boxes, etc): none of these
make sense with ACPI. On the other hand for arm64 distros is a must have.
This is why I think it is a good idea to have it selectable.

I am fine with it being conditional on EXPERT = "y", though.

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 22/22] xen/arm64: Add ACPI support
  2016-03-04  6:15 ` [PATCH v5 22/22] xen/arm64: Add ACPI support Shannon Zhao
  2016-03-04 10:33   ` Jan Beulich
@ 2016-03-04 11:45   ` Stefano Stabellini
  1 sibling, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:45 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, Jan Beulich, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Naresh Bhat <naresh.bhat@linaro.org>
> 
> Add ACPI support on arm64 xen hypervisor. Enable EFI support on ARM.
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


> v5: make ACPI selectable option
> ---
>  xen/arch/arm/Kconfig         |  9 +++++++++
>  xen/common/efi/runtime.c     | 12 +++++++-----
>  xen/include/asm-arm/config.h |  4 ++++
>  3 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index cb99df5..25cec31 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -33,6 +33,15 @@ menu "Architecture Features"
>  
>  source "arch/Kconfig"
>  
> +config ACPI
> +	bool "ACPI (Advanced Configuration and Power Interface) Support"
> +	depends on ARM_64
> +	default y
> +	---help---
> +
> +	  Advanced Configuration and Power Interface (ACPI) support for Xen is
> +	  an alternative to device tree on ARM64.
> +
>  # Select HAS_GICV3 if GICv3 is supported
>  config HAS_GICV3
>  	bool
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index ae87557..c256814 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -10,14 +10,16 @@ DEFINE_XEN_GUEST_HANDLE(CHAR16);
>  
>  #ifndef COMPAT
>  
> -#ifdef CONFIG_ARM  /* Disabled until runtime services implemented */
> -const bool_t efi_enabled = 0;
> -#else
> +/*
> + * Currently runtime services are not implemented on ARM. To boot Xen with ACPI,
> + * set efi_enabled to 1, so that Xen can get the ACPI root pointer from EFI.
> + */
> +const bool_t efi_enabled = 1;
> +
> +#ifndef CONFIG_ARM
>  # include <asm/i387.h>
>  # include <asm/xstate.h>
>  # include <public/platform.h>
> -
> -const bool_t efi_enabled = 1;
>  #endif
>  
>  unsigned int __read_mostly efi_num_ct;
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 7ceb5c5..5fc9aa2 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -31,6 +31,10 @@
>  
>  #define CONFIG_ARM_L1_CACHE_SHIFT 7 /* XXX */
>  
> +#ifdef CONFIG_ACPI
> +#define CONFIG_ACPI_BOOT 1
> +#endif
> +
>  #define CONFIG_SMP 1
>  
>  #define CONFIG_IRQ_HAS_MULTIPLE_ACTION 1
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions
  2016-03-04  6:15 ` [PATCH v5 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions Shannon Zhao
@ 2016-03-04 11:59   ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 11:59 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a new member in gic_hw_operations which is used to deny Dom0 access
> to GIC regions.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  xen/arch/arm/gic-v2.c     | 31 +++++++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/gic.c        |  5 +++++
>  xen/include/asm-arm/gic.h |  3 +++
>  4 files changed, 83 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index d9fce4b..67797f2 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -27,6 +27,7 @@
>  #include <xen/cpu.h>
>  #include <xen/mm.h>
>  #include <xen/irq.h>
> +#include <xen/iocap.h>
>  #include <xen/sched.h>
>  #include <xen/errno.h>
>  #include <xen/delay.h>
> @@ -1278,6 +1279,44 @@ static u32 gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
>      return table_len;
>  }
>  
> +static int gicv3_iomem_deny_access(const struct domain *d)
> +{
> +    int rc, i;
> +    unsigned long gfn, nr;
> +
> +    gfn = dbase >> PAGE_SHIFT;
> +    rc = iomem_deny_access(d, gfn, gfn + 1);

The size of the gicv3 distributor is 64k.

The rest looks good.

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0
  2016-03-04  6:15 ` [PATCH v5 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0 Shannon Zhao
@ 2016-03-04 12:01   ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 12:01 UTC (permalink / raw)
  To: Shannon Zhao; +Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Firstly it permits full MMIO capabilities for Dom0. Then deny MMIO
> access of Xen used devices, such as UART, GIC, SMMU. Currently, it only
> denies the MMIO access of UART and GIC regions. For other Xen used
> devices it could be added later when they are supported.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: deny access to GIC regions

Thank you!

Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  xen/arch/arm/domain_build.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 1e5ee0e..a4abf28 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1359,6 +1359,38 @@ static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>  #ifdef CONFIG_ACPI
>  #define ACPI_DOM0_FDT_MIN_SIZE 4096
>  
> +static int acpi_iomem_deny_access(struct domain *d)
> +{
> +    acpi_status status;
> +    struct acpi_table_spcr *spcr = NULL;
> +    unsigned long gfn;
> +    int rc;
> +
> +    /* Firstly permit full MMIO capabilities. */
> +    rc = iomem_permit_access(d, 0UL, ~0UL);
> +    if ( rc )
> +        return rc;
> +
> +    /* TODO: Deny MMIO access for SMMU, GIC ITS */
> +    status = acpi_get_table(ACPI_SIG_SPCR, 0,
> +                            (struct acpi_table_header **)&spcr);
> +
> +    if ( ACPI_FAILURE(status) )
> +    {
> +        printk("Failed to get SPCR table\n");
> +        return -EINVAL;
> +    }
> +
> +    gfn = spcr->serial_port.address >> PAGE_SHIFT;
> +    /* Deny MMIO access for UART */
> +    rc = iomem_deny_access(d, gfn, gfn + 1);
> +    if ( rc )
> +        return rc;
> +
> +    /* Deny MMIO access for GIC regions */
> +    return gic_iomem_deny_access(d);
> +}
> +
>  static int acpi_permit_spi_access(struct domain *d)
>  {
>      int i, res;
> @@ -1880,6 +1912,10 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>      if ( rc != 0 )
>          return rc;
>  
> +    rc = acpi_iomem_deny_access(d);
> +    if ( rc != 0 )
> +        return rc;
> +
>      return 0;
>  }
>  #else
> -- 
> 2.0.4
> 
> 

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04 10:16   ` Jan Beulich
@ 2016-03-04 12:09     ` Stefano Stabellini
  2016-03-04 12:20       ` Jan Beulich
  2016-03-16 15:03     ` Julien Grall
  1 sibling, 1 reply; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 12:09 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, xen-devel, stefano.stabellini, shannon.zhao, Shannon Zhao

On Fri, 4 Mar 2016, Jan Beulich wrote:
> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > Add a new delivery type:
> > val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
> > To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
> > bit 9 stands the interrupt polarity is active low(1) or high(0).
> > 
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> 
> The set of Cc-s is too narrow - all REST maintainers should be copied.
> 
> > --- a/xen/include/public/hvm/params.h
> > +++ b/xen/include/public/hvm/params.h
> > @@ -55,6 +55,16 @@
> >   * if this delivery method is available.
> >   */
> >  
> > +#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
> > +/*
> > + * val[55:16] need to be zero.
> > + * val[15:8] is flag of event-channel interrupt:
> > + *  bit 8: interrupt is edge(1) or level(0) triggered
> > + *  bit 9: interrupt is active low(1) or high(0)
> > + * val[7:0] is PPI number used by event-channel.
> > + * This is only used by ARM/ARM64.
> > + */
> 
> I think the name of the constant needs improvement. The low 8
> bits make this extremely ARM specific, so perhaps
> HVM_PARAM_CALLBACK_TYPE_PPI?

That would be OK for me.


> Albeit - wouldn't the
> vector and/or GSI ones be re-usable for this purpose? (I
> don't know enough about ARM to be certain.)

Vector is an x86 thing, while GSI is an ACPI term. There is probably a
PPI to GSI mapping on ARM ACPI systems, but I wouldn't want this
HVM_PARAM to work only with ACPI: it should work with device tree
systems too, where GSI has no meaning.


> And then I'm now also wondering about the description of bits
> 8 and 9 - event channels don't know of edge/level triggering
> or high/low polarity, so it looks to me as if the comment is at
> least misleading too.

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04 12:09     ` Stefano Stabellini
@ 2016-03-04 12:20       ` Jan Beulich
  2016-03-04 12:26         ` Stefano Stabellini
  0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2016-03-04 12:20 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: hangaohuai, xen-devel, stefano.stabellini, shannon.zhao, Shannon Zhao

>>> On 04.03.16 at 13:09, <stefano.stabellini@eu.citrix.com> wrote:
> On Fri, 4 Mar 2016, Jan Beulich wrote:
>> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Add a new delivery type:
>> > val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
>> > To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
>> > bit 9 stands the interrupt polarity is active low(1) or high(0).
>> > 
>> > Cc: Jan Beulich <jbeulich@suse.com>
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> 
>> The set of Cc-s is too narrow - all REST maintainers should be copied.
>> 
>> > --- a/xen/include/public/hvm/params.h
>> > +++ b/xen/include/public/hvm/params.h
>> > @@ -55,6 +55,16 @@
>> >   * if this delivery method is available.
>> >   */
>> >  
>> > +#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
>> > +/*
>> > + * val[55:16] need to be zero.
>> > + * val[15:8] is flag of event-channel interrupt:
>> > + *  bit 8: interrupt is edge(1) or level(0) triggered
>> > + *  bit 9: interrupt is active low(1) or high(0)
>> > + * val[7:0] is PPI number used by event-channel.
>> > + * This is only used by ARM/ARM64.
>> > + */
>> 
>> I think the name of the constant needs improvement. The low 8
>> bits make this extremely ARM specific, so perhaps
>> HVM_PARAM_CALLBACK_TYPE_PPI?
> 
> That would be OK for me.
> 
> 
>> Albeit - wouldn't the
>> vector and/or GSI ones be re-usable for this purpose? (I
>> don't know enough about ARM to be certain.)
> 
> Vector is an x86 thing, while GSI is an ACPI term. There is probably a
> PPI to GSI mapping on ARM ACPI systems, but I wouldn't want this
> HVM_PARAM to work only with ACPI: it should work with device tree
> systems too, where GSI has no meaning.

Okay. Then how about making _VECTOR x86-specific, and the
new one ARM-specific (via #ifdef), at once allowing (if so
wanted) both to share the same numeric value?

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04 12:20       ` Jan Beulich
@ 2016-03-04 12:26         ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 12:26 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, Stefano Stabellini, xen-devel, stefano.stabellini,
	shannon.zhao, Shannon Zhao

On Fri, 4 Mar 2016, Jan Beulich wrote:
> >>> On 04.03.16 at 13:09, <stefano.stabellini@eu.citrix.com> wrote:
> > On Fri, 4 Mar 2016, Jan Beulich wrote:
> >> >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> >> > From: Shannon Zhao <shannon.zhao@linaro.org>
> >> > 
> >> > Add a new delivery type:
> >> > val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
> >> > To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
> >> > bit 9 stands the interrupt polarity is active low(1) or high(0).
> >> > 
> >> > Cc: Jan Beulich <jbeulich@suse.com>
> >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> 
> >> The set of Cc-s is too narrow - all REST maintainers should be copied.
> >> 
> >> > --- a/xen/include/public/hvm/params.h
> >> > +++ b/xen/include/public/hvm/params.h
> >> > @@ -55,6 +55,16 @@
> >> >   * if this delivery method is available.
> >> >   */
> >> >  
> >> > +#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
> >> > +/*
> >> > + * val[55:16] need to be zero.
> >> > + * val[15:8] is flag of event-channel interrupt:
> >> > + *  bit 8: interrupt is edge(1) or level(0) triggered
> >> > + *  bit 9: interrupt is active low(1) or high(0)
> >> > + * val[7:0] is PPI number used by event-channel.
> >> > + * This is only used by ARM/ARM64.
> >> > + */
> >> 
> >> I think the name of the constant needs improvement. The low 8
> >> bits make this extremely ARM specific, so perhaps
> >> HVM_PARAM_CALLBACK_TYPE_PPI?
> > 
> > That would be OK for me.
> > 
> > 
> >> Albeit - wouldn't the
> >> vector and/or GSI ones be re-usable for this purpose? (I
> >> don't know enough about ARM to be certain.)
> > 
> > Vector is an x86 thing, while GSI is an ACPI term. There is probably a
> > PPI to GSI mapping on ARM ACPI systems, but I wouldn't want this
> > HVM_PARAM to work only with ACPI: it should work with device tree
> > systems too, where GSI has no meaning.
> 
> Okay. Then how about making _VECTOR x86-specific, and the
> new one ARM-specific (via #ifdef), at once allowing (if so
> wanted) both to share the same numeric value?

Fine by me

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04 10:55     ` Stefano Stabellini
@ 2016-03-04 15:03       ` Shannon Zhao
  2016-03-04 15:23         ` Stefano Stabellini
  2016-03-04 15:39         ` Jan Beulich
  0 siblings, 2 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04 15:03 UTC (permalink / raw)
  To: Stefano Stabellini, Jan Beulich
  Cc: hangaohuai, xen-devel, stefano.stabellini, Shannon Zhao

On 2016年03月04日 18:55, Stefano Stabellini wrote:
> On Fri, 4 Mar 2016, Jan Beulich wrote:
>>>>> > >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
>>> > > From: Shannon Zhao <shannon.zhao@linaro.org>
>>> > > 
>>> > > Estimate the memory required for loading acpi/efi tables in Dom0. Make
>>> > > the length of each table aligned with 64bit. Alloc the pages to store
>>> > > the new created EFI and ACPI tables and free these pages when
>>> > > destroying domain.
>>> > > 
>>> > > Cc: Jan Beulich <jbeulich@suse.com>
>>> > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>>> > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Since the pattern repeats I finally have to ask: Who is the author
>> > of a patch with such a set of tag? You (From:) or Parth (first S-o-b)?
>> > 
>>> > > --- a/xen/common/efi/boot.c
>>> > > +++ b/xen/common/efi/boot.c
>>> > > @@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>>> > >      for( ; ; ); /* not reached */
>>> > >  }
>>> > >  
>>> > > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
>>> > > +struct meminfo __init *get_acpi_meminfo(void)
>>> > > +{
>>> > > +    return &acpi_mem;
>>> > > +}
>>> > > +#endif
>> > 
>> > No such hackery in common code please, if at all avoidable. If ARM
>> > maintainers are fine with this in their code, it could be put into
>> > ARM's efi-boot.h.
> I am OK with that. If you move it under arch/arm, then drop the ifdef
> CONFIG_ARM.
> 
While I want to include efi-boot.h in efi-dom.c firstly, but there are
many defined-but-not-used errors because there are some of functions in
efi-boot.h which are not used in efi-dom0.c.
So how could we solve such a problem?

Thanks,
--
Shannon

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset
  2016-03-04 10:59   ` Stefano Stabellini
@ 2016-03-04 15:12     ` Shannon Zhao
  2016-03-04 15:31       ` Stefano Stabellini
  0 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04 15:12 UTC (permalink / raw)
  To: Stefano Stabellini, Shannon Zhao
  Cc: hangaohuai, stefano.stabellini, xen-devel

On 2016年03月04日 18:59, Stefano Stabellini wrote:
>> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
>> > index 7f59761..6db3711 100644
>> > --- a/xen/include/asm-arm/acpi.h
>> > +++ b/xen/include/asm-arm/acpi.h
>> > @@ -25,6 +25,7 @@
>> >  
>> >  #include <xen/init.h>
>> >  #include <asm/page.h>
>> > +#include <asm/setup.h>
>> >  
>> >  #define COMPILER_DEPENDENT_INT64   long long
>> >  #define COMPILER_DEPENDENT_UINT64  unsigned long long
>> > @@ -58,10 +59,15 @@ static inline void enable_acpi(void)
>> >  {
>> >      acpi_disabled = 0;
>> >  }
>> > +
>> > +paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index);
>> >  #else
>> >  #define acpi_disabled (1)
>> >  #define disable_acpi()
>> >  #define enable_acpi()
>> > +paddr_t inline acpi_get_table_offset(struct membank tbl_add[],
>> > +                                     EFI_MEM_RES index)
>> > +{ return 0; }
> Why did you move the declaration of acpi_get_table_offset to within the
> ifdef?  It was just above the ifdef in the previous version of the
> patch.
> 
If we don't do this, when we compile Xen just only apply the first 11
patches of this series and since the acpi_create_efi_system_table is not
covered by #ifdef CONFIG_ACPI, it will throw out an error says something
like "acpi_get_table_offset not defined".

Thanks,
-- 
Shannon

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically
  2016-03-04 11:26   ` Stefano Stabellini
@ 2016-03-04 15:16     ` Shannon Zhao
  0 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04 15:16 UTC (permalink / raw)
  To: Stefano Stabellini, Shannon Zhao
  Cc: hangaohuai, stefano.stabellini, xen-devel

On 2016年03月04日 19:26, Stefano Stabellini wrote:
> On Fri, 4 Mar 2016, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Interrupt information is described in DSDT and is not available at the
>> > time of booting. Check if the interrupt is permitted to access and set
>> > the interrupt type, route it to guest dynamically only for SPI
>> > and Dom0.
>> > 
>> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> > v5: add a small function to get the irq type from ICFG
>> > ---
>> >  xen/arch/arm/vgic.c | 38 ++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 38 insertions(+)
>> > 
>> > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
>> > index ee35683..52378a3 100644
>> > --- a/xen/arch/arm/vgic.c
>> > +++ b/xen/arch/arm/vgic.c
>> > @@ -25,6 +25,8 @@
>> >  #include <xen/irq.h>
>> >  #include <xen/sched.h>
>> >  #include <xen/perfc.h>
>> > +#include <xen/iocap.h>
>> > +#include <xen/acpi.h>
>> >  
>> >  #include <asm/current.h>
>> >  
>> > @@ -334,6 +336,21 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
>> >      }
>> >  }
>> >  
>> > +#ifdef CONFIG_ACPI
>> > +#define VGIC_ICFG_MASK(intr) ( 1 << ( ( 2 * ( intr % 16 ) ) + 1 ) )
>                   remove spaces: ^      ^                   ^       ^
> 
>> > +static unsigned int get_the_irq_type(struct vcpu *v, int n, int index)
> static inline
> 
> 
>> > +{
>> > +    struct vgic_irq_rank *vr = vgic_get_rank(v, n);
>> > +    uint32_t tr = vr->icfg[index >> 4];
>> > +
>> > +    if ( tr & VGIC_ICFG_MASK(index) )
>> > +        return IRQ_TYPE_EDGE_BOTH;
>> > +    else
>> > +        return IRQ_TYPE_LEVEL_MASK;
>> > +}
>> > +#endif
>> > +
>> >  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>> >  {
>> >      const unsigned long mask = r;
>> > @@ -342,9 +359,30 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>> >      unsigned long flags;
>> >      int i = 0;
>> >      struct vcpu *v_target;
>> > +#ifdef CONFIG_ACPI
>> > +    struct domain *d = v->domain;
>> > +    int ret;
>> > +#endif
>> >  
>> >      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>> >          irq = i + (32 * n);
>> > +#ifdef CONFIG_ACPI
>> > +        /* Set the irq type and route it to guest only for SPI and Dom0 */
>> > +        if( irq_access_permitted(d, irq) && is_hardware_domain(d) &&
>> > +            ( irq >= 32 ) && ( !acpi_disabled ) )
> Is there a reason why we need to have this code inside an ifdef? It
> looks like we can remove all ifdefs from this patch.
> 
> 
Ah, right! Will remove it.

Thanks,
-- 
Shannon

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04 15:03       ` Shannon Zhao
@ 2016-03-04 15:23         ` Stefano Stabellini
  2016-03-04 15:52           ` Shannon Zhao
  2016-03-04 15:39         ` Jan Beulich
  1 sibling, 1 reply; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 15:23 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: hangaohuai, Stefano Stabellini, xen-devel, stefano.stabellini,
	Jan Beulich, Shannon Zhao

[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> On 2016年03月04日 18:55, Stefano Stabellini wrote:
> > On Fri, 4 Mar 2016, Jan Beulich wrote:
> >>>>> > >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> >>> > > From: Shannon Zhao <shannon.zhao@linaro.org>
> >>> > > 
> >>> > > Estimate the memory required for loading acpi/efi tables in Dom0. Make
> >>> > > the length of each table aligned with 64bit. Alloc the pages to store
> >>> > > the new created EFI and ACPI tables and free these pages when
> >>> > > destroying domain.
> >>> > > 
> >>> > > Cc: Jan Beulich <jbeulich@suse.com>
> >>> > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
> >>> > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> > 
> >> > Since the pattern repeats I finally have to ask: Who is the author
> >> > of a patch with such a set of tag? You (From:) or Parth (first S-o-b)?
> >> > 
> >>> > > --- a/xen/common/efi/boot.c
> >>> > > +++ b/xen/common/efi/boot.c
> >>> > > @@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
> >>> > >      for( ; ; ); /* not reached */
> >>> > >  }
> >>> > >  
> >>> > > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
> >>> > > +struct meminfo __init *get_acpi_meminfo(void)
> >>> > > +{
> >>> > > +    return &acpi_mem;
> >>> > > +}
> >>> > > +#endif
> >> > 
> >> > No such hackery in common code please, if at all avoidable. If ARM
> >> > maintainers are fine with this in their code, it could be put into
> >> > ARM's efi-boot.h.
> > I am OK with that. If you move it under arch/arm, then drop the ifdef
> > CONFIG_ARM.
> > 
> While I want to include efi-boot.h in efi-dom.c firstly, but there are
> many defined-but-not-used errors because there are some of functions in
> efi-boot.h which are not used in efi-dom0.c.
> So how could we solve such a problem?
 
It is probably not a good idea to include efi-boot.h in efi-dom.c. Why
are you trying to do that? 

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset
  2016-03-04 15:12     ` Shannon Zhao
@ 2016-03-04 15:31       ` Stefano Stabellini
  2016-03-04 15:53         ` Shannon Zhao
  0 siblings, 1 reply; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 15:31 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: stefano.stabellini, hangaohuai, Shannon Zhao, xen-devel,
	Stefano Stabellini

[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> On 2016年03月04日 18:59, Stefano Stabellini wrote:
> >> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
> >> > index 7f59761..6db3711 100644
> >> > --- a/xen/include/asm-arm/acpi.h
> >> > +++ b/xen/include/asm-arm/acpi.h
> >> > @@ -25,6 +25,7 @@
> >> >  
> >> >  #include <xen/init.h>
> >> >  #include <asm/page.h>
> >> > +#include <asm/setup.h>
> >> >  
> >> >  #define COMPILER_DEPENDENT_INT64   long long
> >> >  #define COMPILER_DEPENDENT_UINT64  unsigned long long
> >> > @@ -58,10 +59,15 @@ static inline void enable_acpi(void)
> >> >  {
> >> >      acpi_disabled = 0;
> >> >  }
> >> > +
> >> > +paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index);
> >> >  #else
> >> >  #define acpi_disabled (1)
> >> >  #define disable_acpi()
> >> >  #define enable_acpi()
> >> > +paddr_t inline acpi_get_table_offset(struct membank tbl_add[],
> >> > +                                     EFI_MEM_RES index)
> >> > +{ return 0; }
> > Why did you move the declaration of acpi_get_table_offset to within the
> > ifdef?  It was just above the ifdef in the previous version of the
> > patch.
> > 
> If we don't do this, when we compile Xen just only apply the first 11
> patches of this series and since the acpi_create_efi_system_table is not
> covered by #ifdef CONFIG_ACPI, it will throw out an error says something
> like "acpi_get_table_offset not defined".

Just build efi-dom0 only if CONFIG_ACPI, then you can move back the
declaration of acpi_get_table_offset where is was before:

diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
index b38a0c9..d34c916 100644
--- a/xen/arch/arm/efi/Makefile
+++ b/xen/arch/arm/efi/Makefile
@@ -1,3 +1,4 @@
 CFLAGS += -fshort-wchar
 
-obj-y +=  boot.init.o runtime.o efi-dom0.init.o
+obj-y +=  boot.init.o runtime.o
+obj-$(CONFIG_ACPI) +=  efi-dom0.init.o

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04 15:03       ` Shannon Zhao
  2016-03-04 15:23         ` Stefano Stabellini
@ 2016-03-04 15:39         ` Jan Beulich
  1 sibling, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2016-03-04 15:39 UTC (permalink / raw)
  To: Stefano Stabellini, Shannon Zhao
  Cc: hangaohuai, xen-devel, stefano.stabellini, Shannon Zhao

>>> On 04.03.16 at 16:03, <shannon.zhao@linaro.org> wrote:
> On 2016年03月04日 18:55, Stefano Stabellini wrote:
>> On Fri, 4 Mar 2016, Jan Beulich wrote:
>>>>>> > >>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
>>>> > > From: Shannon Zhao <shannon.zhao@linaro.org>
>>>> > > 
>>>> > > Estimate the memory required for loading acpi/efi tables in Dom0. Make
>>>> > > the length of each table aligned with 64bit. Alloc the pages to store
>>>> > > the new created EFI and ACPI tables and free these pages when
>>>> > > destroying domain.
>>>> > > 
>>>> > > Cc: Jan Beulich <jbeulich@suse.com>
>>>> > > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>>>> > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> > 
>>> > Since the pattern repeats I finally have to ask: Who is the author
>>> > of a patch with such a set of tag? You (From:) or Parth (first S-o-b)?
>>> > 
>>>> > > --- a/xen/common/efi/boot.c
>>>> > > +++ b/xen/common/efi/boot.c
>>>> > > @@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
> *SystemTable)
>>>> > >      for( ; ; ); /* not reached */
>>>> > >  }
>>>> > >  
>>>> > > +#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
>>>> > > +struct meminfo __init *get_acpi_meminfo(void)
>>>> > > +{
>>>> > > +    return &acpi_mem;
>>>> > > +}
>>>> > > +#endif
>>> > 
>>> > No such hackery in common code please, if at all avoidable. If ARM
>>> > maintainers are fine with this in their code, it could be put into
>>> > ARM's efi-boot.h.
>> I am OK with that. If you move it under arch/arm, then drop the ifdef
>> CONFIG_ARM.
>> 
> While I want to include efi-boot.h in efi-dom.c firstly, but there are
> many defined-but-not-used errors because there are some of functions in
> efi-boot.h which are not used in efi-dom0.c.
> So how could we solve such a problem?

efi-boot.h is not a normal header, and should not be included by
other than efi/boot.c. Just put the declaration in some header
under asm-arm/.

Jan

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04 15:23         ` Stefano Stabellini
@ 2016-03-04 15:52           ` Shannon Zhao
  2016-03-04 15:59             ` Stefano Stabellini
  0 siblings, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04 15:52 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: hangaohuai, xen-devel, stefano.stabellini, Jan Beulich, Shannon Zhao



On 2016/3/4 23:23, Stefano Stabellini wrote:
> On Fri, 4 Mar 2016, Shannon Zhao wrote:
>> >On 2016年03月04日 18:55, Stefano Stabellini wrote:
>>> > >On Fri, 4 Mar 2016, Jan Beulich wrote:
>>>>>>>>>>> > >>>>> > >>>On 04.03.16 at 07:15,<zhaoshenglong@huawei.com>  wrote:
>>>>>>> > >>> > >From: Shannon Zhao<shannon.zhao@linaro.org>
>>>>>>> > >>> > >
>>>>>>> > >>> > >Estimate the memory required for loading acpi/efi tables in Dom0. Make
>>>>>>> > >>> > >the length of each table aligned with 64bit. Alloc the pages to store
>>>>>>> > >>> > >the new created EFI and ACPI tables and free these pages when
>>>>>>> > >>> > >destroying domain.
>>>>>>> > >>> > >
>>>>>>> > >>> > >Cc: Jan Beulich<jbeulich@suse.com>
>>>>>>> > >>> > >Signed-off-by: Parth Dixit<parth.dixit@linaro.org>
>>>>>>> > >>> > >Signed-off-by: Shannon Zhao<shannon.zhao@linaro.org>
>>>>> > >> >
>>>>> > >> >Since the pattern repeats I finally have to ask: Who is the author
>>>>> > >> >of a patch with such a set of tag? You (From:) or Parth (first S-o-b)?
>>>>> > >> >
>>>>>>> > >>> > >--- a/xen/common/efi/boot.c
>>>>>>> > >>> > >+++ b/xen/common/efi/boot.c
>>>>>>> > >>> > >@@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
>>>>>>> > >>> > >      for( ; ; ); /* not reached */
>>>>>>> > >>> > >  }
>>>>>>> > >>> > >
>>>>>>> > >>> > >+#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
>>>>>>> > >>> > >+struct meminfo __init *get_acpi_meminfo(void)
>>>>>>> > >>> > >+{
>>>>>>> > >>> > >+    return &acpi_mem;
>>>>>>> > >>> > >+}
>>>>>>> > >>> > >+#endif
>>>>> > >> >
>>>>> > >> >No such hackery in common code please, if at all avoidable. If ARM
>>>>> > >> >maintainers are fine with this in their code, it could be put into
>>>>> > >> >ARM's efi-boot.h.
>>> > >I am OK with that. If you move it under arch/arm, then drop the ifdef
>>> > >CONFIG_ARM.
>>> > >
>> >While I want to include efi-boot.h in efi-dom.c firstly, but there are
>> >many defined-but-not-used errors because there are some of functions in
>> >efi-boot.h which are not used in efi-dom0.c.
>> >So how could we solve such a problem?
>
> It is probably not a good idea to include efi-boot.h in efi-dom.c. Why
> are you trying to do that?
Use acpi_mem which is defined in efi-boot.h. Maybe as Jan suggested I 
should add a new header under asm-arm to extern acpi_mem then include 
this new header to efi-dom0.c. How about this?

Thanks,
-- 
Shannon

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset
  2016-03-04 15:31       ` Stefano Stabellini
@ 2016-03-04 15:53         ` Shannon Zhao
  0 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-04 15:53 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: hangaohuai, xen-devel, stefano.stabellini, Shannon Zhao



On 2016/3/4 23:31, Stefano Stabellini wrote:
> On Fri, 4 Mar 2016, Shannon Zhao wrote:
>> >On 2016年03月04日 18:59, Stefano Stabellini wrote:
>>>> > >>diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
>>>>> > >> >index 7f59761..6db3711 100644
>>>>> > >> >--- a/xen/include/asm-arm/acpi.h
>>>>> > >> >+++ b/xen/include/asm-arm/acpi.h
>>>>> > >> >@@ -25,6 +25,7 @@
>>>>> > >> >
>>>>> > >> >  #include <xen/init.h>
>>>>> > >> >  #include <asm/page.h>
>>>>> > >> >+#include <asm/setup.h>
>>>>> > >> >
>>>>> > >> >  #define COMPILER_DEPENDENT_INT64   long long
>>>>> > >> >  #define COMPILER_DEPENDENT_UINT64  unsigned long long
>>>>> > >> >@@ -58,10 +59,15 @@ static inline void enable_acpi(void)
>>>>> > >> >  {
>>>>> > >> >      acpi_disabled = 0;
>>>>> > >> >  }
>>>>> > >> >+
>>>>> > >> >+paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index);
>>>>> > >> >  #else
>>>>> > >> >  #define acpi_disabled (1)
>>>>> > >> >  #define disable_acpi()
>>>>> > >> >  #define enable_acpi()
>>>>> > >> >+paddr_t inline acpi_get_table_offset(struct membank tbl_add[],
>>>>> > >> >+                                     EFI_MEM_RES index)
>>>>> > >> >+{ return 0; }
>>> > >Why did you move the declaration of acpi_get_table_offset to within the
>>> > >ifdef?  It was just above the ifdef in the previous version of the
>>> > >patch.
>>> > >
>> >If we don't do this, when we compile Xen just only apply the first 11
>> >patches of this series and since the acpi_create_efi_system_table is not
>> >covered by #ifdef CONFIG_ACPI, it will throw out an error says something
>> >like "acpi_get_table_offset not defined".
> Just build efi-dom0 only if CONFIG_ACPI, then you can move back the
> declaration of acpi_get_table_offset where is was before:
>
> diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
> index b38a0c9..d34c916 100644
> --- a/xen/arch/arm/efi/Makefile
> +++ b/xen/arch/arm/efi/Makefile
> @@ -1,3 +1,4 @@
>   CFLAGS += -fshort-wchar
>
> -obj-y +=  boot.init.o runtime.o efi-dom0.init.o
> +obj-y +=  boot.init.o runtime.o
> +obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
Ok. Will do.

Thanks,
-- 
Shannon

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables
  2016-03-04 15:52           ` Shannon Zhao
@ 2016-03-04 15:59             ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-04 15:59 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: hangaohuai, Stefano Stabellini, xen-devel, stefano.stabellini,
	Jan Beulich, Shannon Zhao

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

On Fri, 4 Mar 2016, Shannon Zhao wrote:
> On 2016/3/4 23:23, Stefano Stabellini wrote:
> > On Fri, 4 Mar 2016, Shannon Zhao wrote:
> > > >On 2016年03月04日 18:55, Stefano Stabellini wrote:
> > > > > >On Fri, 4 Mar 2016, Jan Beulich wrote:
> > > > > > > > > > > > > >>>>> > >>>On 04.03.16 at
> > > > > > > > > > > > 07:15,<zhaoshenglong@huawei.com>  wrote:
> > > > > > > > > >>> > >From: Shannon Zhao<shannon.zhao@linaro.org>
> > > > > > > > > >>> > >
> > > > > > > > > >>> > >Estimate the memory required for loading acpi/efi
> > > > > > > > tables in Dom0. Make
> > > > > > > > > >>> > >the length of each table aligned with 64bit. Alloc the
> > > > > > > > pages to store
> > > > > > > > > >>> > >the new created EFI and ACPI tables and free these
> > > > > > > > pages when
> > > > > > > > > >>> > >destroying domain.
> > > > > > > > > >>> > >
> > > > > > > > > >>> > >Cc: Jan Beulich<jbeulich@suse.com>
> > > > > > > > > >>> > >Signed-off-by: Parth Dixit<parth.dixit@linaro.org>
> > > > > > > > > >>> > >Signed-off-by: Shannon Zhao<shannon.zhao@linaro.org>
> > > > > > > >> >
> > > > > > > >> >Since the pattern repeats I finally have to ask: Who is the
> > > > > > author
> > > > > > > >> >of a patch with such a set of tag? You (From:) or Parth (first
> > > > > > S-o-b)?
> > > > > > > >> >
> > > > > > > > > >>> > >--- a/xen/common/efi/boot.c
> > > > > > > > > >>> > >+++ b/xen/common/efi/boot.c
> > > > > > > > > >>> > >@@ -1151,6 +1151,13 @@ efi_start(EFI_HANDLE
> > > > > > > > ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
> > > > > > > > > >>> > >      for( ; ; ); /* not reached */
> > > > > > > > > >>> > >  }
> > > > > > > > > >>> > >
> > > > > > > > > >>> > >+#if defined (CONFIG_ACPI) && defined (CONFIG_ARM)
> > > > > > > > > >>> > >+struct meminfo __init *get_acpi_meminfo(void)
> > > > > > > > > >>> > >+{
> > > > > > > > > >>> > >+    return &acpi_mem;
> > > > > > > > > >>> > >+}
> > > > > > > > > >>> > >+#endif
> > > > > > > >> >
> > > > > > > >> >No such hackery in common code please, if at all avoidable. If
> > > > > > ARM
> > > > > > > >> >maintainers are fine with this in their code, it could be put
> > > > > > into
> > > > > > > >> >ARM's efi-boot.h.
> > > > > >I am OK with that. If you move it under arch/arm, then drop the ifdef
> > > > > >CONFIG_ARM.
> > > > > >
> > > >While I want to include efi-boot.h in efi-dom.c firstly, but there are
> > > >many defined-but-not-used errors because there are some of functions in
> > > >efi-boot.h which are not used in efi-dom0.c.
> > > >So how could we solve such a problem?
> > 
> > It is probably not a good idea to include efi-boot.h in efi-dom.c. Why
> > are you trying to do that?
> Use acpi_mem which is defined in efi-boot.h. Maybe as Jan suggested I should
> add a new header under asm-arm to extern acpi_mem then include this new header
> to efi-dom0.c. How about this?

Yes, that's how I think it should be done.

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04  6:15 ` [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ Shannon Zhao
  2016-03-04 10:16   ` Jan Beulich
@ 2016-03-04 21:19   ` Konrad Rzeszutek Wilk
  2016-03-16 16:34     ` Julien Grall
  1 sibling, 1 reply; 63+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-04 21:19 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, Jan Beulich, xen-devel

On Fri, Mar 04, 2016 at 02:15:49PM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add a new delivery type:
> val[63:56] == 3: val[15:8] is flag: val[7:0] is a PPI.
> To the flag, bit 8 stands the interrupt mode is edge(1) or level(0) and
> bit 9 stands the interrupt polarity is active low(1) or high(0).
> 
> Cc: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v5: fix the statement
> ---
>  xen/include/public/hvm/params.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index 73d4718..2ea8d62 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -55,6 +55,16 @@
>   * if this delivery method is available.
>   */
>  
> +#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
> +/*
> + * val[55:16] need to be zero.
> + * val[15:8] is flag of event-channel interrupt:
> + *  bit 8: interrupt is edge(1) or level(0) triggered
> + *  bit 9: interrupt is active low(1) or high(0)
> + * val[7:0] is PPI number used by event-channel.
> + * This is only used by ARM/ARM64.


How do you deal with masking and EOI?

One of the things that I have realized when looking at vAPIC, HVMOP_set_evtchn_upcall_vector,
and this vector callback is that it is a bit ... wild west.

On x86 when we inject the vector - the Linux (and FreeBSD) call right in
the event channel code. Both of them clear the evtchn_upcall_pending right
away but do not set evtchn_upcall_mask.

That means if the hypervisor is told to set another event (via EVTCHNOP_SEND) that
is say more than 64 bits away from the other prior one (so that the
check for evtchn_pending_sel succeeds) it can do so (see evtchn_2l_set_pending
and vcpu_mark_events_pending).

And then we can go on sending an IPI, causing the guest an VMEXIT, and
reinjection of the vector callback. ...

I think if the events are spaced just right you can do this injection up to
16 times. Granted the 16th callback will work just fine and work on all
the events and then EOI all of them. And the other 15 will just exit out
since evtchn_pending_sel will be zero.

But it is a nuisance.

Anyhow what I am wondering if there are some semantincs when it comes to PPI
and it being able to 'mask' an vector until it exits or such? If so
you should document that.

[Yes, I am going to send a patch for the the above 'interesting' behavior].

>  /*
>   * These are not used by Xen. They are here for convenience of HVM-guest
>   * xenbus implementations.
> -- 
> 2.0.4
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0
  2016-03-04 11:13   ` Stefano Stabellini
@ 2016-03-16  8:59     ` Shannon Zhao
  0 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-16  8:59 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel



On 2016/3/4 19:13, Stefano Stabellini wrote:
> On Fri, 4 Mar 2016, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Create a few EFI memory descriptors to tell Dom0 the RAM region
>> > information, ACPI table regions and EFI tables reserved resions.
>> > 
>> > Signed-off-by: Parth Dixit <parth.dixit@linaro.org>
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> > v5: move to efi-dom0.c
>> > ---
>> >  xen/arch/arm/domain_build.c |  2 ++
>> >  xen/arch/arm/efi/efi-dom0.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
>> >  xen/include/asm-arm/setup.h |  5 +++++
>> >  3 files changed, 54 insertions(+)
>> > 
>> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> > index 613551c..008fc76 100644
>> > --- a/xen/arch/arm/domain_build.c
>> > +++ b/xen/arch/arm/domain_build.c
>> > @@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
>> >      acpi_map_other_tables(d);
>> >      acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
>> >                                   tbl_add);
>> > +    acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
>> > +                               d->arch.efi_acpi_table, &kinfo->mem, tbl_add);
>> >  
>> >      return 0;
>> >  }
>> > diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
>> > index 36a1283..0ff6309 100644
>> > --- a/xen/arch/arm/efi/efi-dom0.c
>> > +++ b/xen/arch/arm/efi/efi-dom0.c
>> > @@ -22,6 +22,7 @@
>> >   */
>> >  
>> >  #include "efi.h"
>> > +#include <xen/pfn.h>
>> >  #include <asm/setup.h>
>> >  #include <asm/acpi.h>
>> >  
>> > @@ -90,3 +91,49 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
>> >      tbl_add[TBL_EFIT].start = table_addr;
>> >      tbl_add[TBL_EFIT].size = table_size;
>> >  }
>> > +
>> > +void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
>> > +                                       void *efi_acpi_table,
>> > +                                       const struct meminfo *mem,
>> > +                                       struct membank tbl_add[])
>> > +{
>> > +    EFI_MEMORY_DESCRIPTOR *memory_map;
>> > +    struct meminfo *acpi_mem;
>> > +    int acpi_mem_nr_banks = 0;
>> > +    unsigned int i, offset;
>> > +    u8 *base_ptr;
>> > +
>> > +    base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP);
>> > +    memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr);
>> > +
>> > +    offset = 0;
>> > +    for( i = 0; i < mem->nr_banks; i++, offset++ )
>> > +    {
>> > +        memory_map[offset].Type = EfiConventionalMemory;
>> > +        memory_map[offset].PhysicalStart = mem->bank[i].start;
>> > +        memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size);
>> > +        memory_map[offset].Attribute = EFI_MEMORY_WB;
>> > +    }
>> > +
>> > +    if ( !acpi_disabled )
>> > +    {
> Isn't this check redundant, givem that the function is name is
> acpi_create_efi_mmap_table and is called by prepare_acpi?
> 
Will remove it.

Thanks,
-- 
Shannon


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-04 10:29   ` Jan Beulich
  2016-03-04 11:00     ` Roger Pau Monné
@ 2016-03-16  9:48     ` Shannon Zhao
  2016-03-16 10:04       ` Jan Beulich
  1 sibling, 1 reply; 63+ messages in thread
From: Shannon Zhao @ 2016-03-16  9:48 UTC (permalink / raw)
  To: Jan Beulich, Roger Pau Monne, Boris Ostrovsky
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel



On 2016/3/4 18:29, Jan Beulich wrote:
>> > --- a/xen/arch/arm/mm.c
>> > +++ b/xen/arch/arm/mm.c
>> > @@ -1138,6 +1138,9 @@ int xenmem_add_to_physmap_one(
>> >          rcu_unlock_domain(od);
>> >          break;
>> >      }
>> > +    case XENMAPSPACE_dev_mmio:
>> > +        rc = map_dev_mmio_region(d, gpfn, 1, idx);
> This being the only caller, ...
> 
>> > +int map_dev_mmio_region(struct domain *d,
>> > +                        unsigned long start_gfn,
>> > +                        unsigned long nr,
>> > +                        unsigned long mfn)
>> > +{
> ... what's the "nr" parameter good for? 
While currently the caller passes const value 1, but I think it's fine
to make this function support to map multiple pages for future possible use.

Thanks,
-- 
Shannon


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-16  9:48     ` Shannon Zhao
@ 2016-03-16 10:04       ` Jan Beulich
  2016-03-16 11:22         ` Shannon Zhao
  0 siblings, 1 reply; 63+ messages in thread
From: Jan Beulich @ 2016-03-16 10:04 UTC (permalink / raw)
  To: Shannon Zhao
  Cc: hangaohuai, xen-devel, Julien Grall, stefano.stabellini,
	shannon.zhao, Boris Ostrovsky, Roger Pau Monne

>>> On 16.03.16 at 10:48, <zhaoshenglong@huawei.com> wrote:
> On 2016/3/4 18:29, Jan Beulich wrote:
>>> > --- a/xen/arch/arm/mm.c
>>> > +++ b/xen/arch/arm/mm.c
>>> > @@ -1138,6 +1138,9 @@ int xenmem_add_to_physmap_one(
>>> >          rcu_unlock_domain(od);
>>> >          break;
>>> >      }
>>> > +    case XENMAPSPACE_dev_mmio:
>>> > +        rc = map_dev_mmio_region(d, gpfn, 1, idx);
>> This being the only caller, ...
>> 
>>> > +int map_dev_mmio_region(struct domain *d,
>>> > +                        unsigned long start_gfn,
>>> > +                        unsigned long nr,
>>> > +                        unsigned long mfn)
>>> > +{
>> ... what's the "nr" parameter good for? 
> While currently the caller passes const value 1, but I think it's fine
> to make this function support to map multiple pages for future possible use.

Well, it was just a remark. The maintainers will judge. Looking at the
patch again I notice though that this

+    if(!iomem_access_permitted(d, start_gfn, start_gfn + nr))
+        return 0;

is wrong (and not just malformed) - the range here is an inclusive
one, i.e. you need to subtract one on the right side (and be sure
nr is not zero).

Also please note regarding

+        printk(XENLOG_ERR "Unable to map 0x%lx - 0x%lx in domain %d\n",

that at least in common and x86 code %#lx is preferred over
0x%lx as being a one byte shorter string literal with no meaningful
difference to the produced output. I'd also recommend using
mathematical range representation, to make it clear to the reader
whether the range is inclusive or exclusive (i.e. use either
[<low>,<high>) or [<low>,<high>]), and Dom%d or dom%d.

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping
  2016-03-16 10:04       ` Jan Beulich
@ 2016-03-16 11:22         ` Shannon Zhao
  0 siblings, 0 replies; 63+ messages in thread
From: Shannon Zhao @ 2016-03-16 11:22 UTC (permalink / raw)
  To: Jan Beulich
  Cc: hangaohuai, xen-devel, Julien Grall, stefano.stabellini,
	shannon.zhao, Boris Ostrovsky, Roger Pau Monne



On 2016/3/16 18:04, Jan Beulich wrote:
>>>> On 16.03.16 at 10:48, <zhaoshenglong@huawei.com> wrote:
>> On 2016/3/4 18:29, Jan Beulich wrote:
>>>>> --- a/xen/arch/arm/mm.c
>>>>> +++ b/xen/arch/arm/mm.c
>>>>> @@ -1138,6 +1138,9 @@ int xenmem_add_to_physmap_one(
>>>>>          rcu_unlock_domain(od);
>>>>>          break;
>>>>>      }
>>>>> +    case XENMAPSPACE_dev_mmio:
>>>>> +        rc = map_dev_mmio_region(d, gpfn, 1, idx);
>>> This being the only caller, ...
>>>
>>>>> +int map_dev_mmio_region(struct domain *d,
>>>>> +                        unsigned long start_gfn,
>>>>> +                        unsigned long nr,
>>>>> +                        unsigned long mfn)
>>>>> +{
>>> ... what's the "nr" parameter good for? 
>> While currently the caller passes const value 1, but I think it's fine
>> to make this function support to map multiple pages for future possible use.
> 
> Well, it was just a remark. The maintainers will judge. Looking at the
> patch again I notice though that this
> 
> +    if(!iomem_access_permitted(d, start_gfn, start_gfn + nr))
> +        return 0;
> 
> is wrong (and not just malformed) - the range here is an inclusive
> one, i.e. you need to subtract one on the right side (and be sure
> nr is not zero).
> 
Ah, right! Will fix this.

> Also please note regarding
> 
> +        printk(XENLOG_ERR "Unable to map 0x%lx - 0x%lx in domain %d\n",
> 
> that at least in common and x86 code %#lx is preferred over
> 0x%lx as being a one byte shorter string literal with no meaningful
> difference to the produced output. I'd also recommend using
> mathematical range representation, to make it clear to the reader
> whether the range is inclusive or exclusive (i.e. use either
> [<low>,<high>) or [<low>,<high>]), and Dom%d or dom%d.
> 
Ok, thanks!

-- 
Shannon


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04 10:16   ` Jan Beulich
  2016-03-04 12:09     ` Stefano Stabellini
@ 2016-03-16 15:03     ` Julien Grall
  2016-03-16 15:10       ` Jan Beulich
  1 sibling, 1 reply; 63+ messages in thread
From: Julien Grall @ 2016-03-16 15:03 UTC (permalink / raw)
  To: Jan Beulich, Shannon Zhao
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

Hi Jan,

On 04/03/2016 10:16, Jan Beulich wrote:
>>>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:

[...]

>> --- a/xen/include/public/hvm/params.h
>> +++ b/xen/include/public/hvm/params.h
>> @@ -55,6 +55,16 @@
>>    * if this delivery method is available.
>>    */
>>
>> +#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
>> +/*
>> + * val[55:16] need to be zero.
>> + * val[15:8] is flag of event-channel interrupt:
>> + *  bit 8: interrupt is edge(1) or level(0) triggered
>> + *  bit 9: interrupt is active low(1) or high(0)
>> + * val[7:0] is PPI number used by event-channel.

is a PPI

>> + * This is only used by ARM/ARM64.
>> + */

[...]

> And then I'm now also wondering about the description of bits
> 8 and 9 - event channels don't know of edge/level triggering
> or high/low polarity, so it looks to me as if the comment is at
> least misleading too.

bit 8 and bit 9 are related to the PPI configuration. We need to know if 
the interrupt is level/edge trigger active low/high to configure 
correctly the interrupt controller.

What about renaming "interrupt" to "PPI" to make clear is related to the 
PPI?

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-16 15:03     ` Julien Grall
@ 2016-03-16 15:10       ` Jan Beulich
  0 siblings, 0 replies; 63+ messages in thread
From: Jan Beulich @ 2016-03-16 15:10 UTC (permalink / raw)
  To: Julien Grall, Shannon Zhao
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, xen-devel

>>> On 16.03.16 at 16:03, <julien.grall@arm.com> wrote:
> On 04/03/2016 10:16, Jan Beulich wrote:
>>>>> On 04.03.16 at 07:15, <zhaoshenglong@huawei.com> wrote:
> 
> [...]
> 
>>> --- a/xen/include/public/hvm/params.h
>>> +++ b/xen/include/public/hvm/params.h
>>> @@ -55,6 +55,16 @@
>>>    * if this delivery method is available.
>>>    */
>>>
>>> +#define HVM_PARAM_CALLBACK_TYPE_EVENT    3
>>> +/*
>>> + * val[55:16] need to be zero.
>>> + * val[15:8] is flag of event-channel interrupt:
>>> + *  bit 8: interrupt is edge(1) or level(0) triggered
>>> + *  bit 9: interrupt is active low(1) or high(0)
>>> + * val[7:0] is PPI number used by event-channel.
> 
> is a PPI
> 
>>> + * This is only used by ARM/ARM64.
>>> + */
> 
> [...]
> 
>> And then I'm now also wondering about the description of bits
>> 8 and 9 - event channels don't know of edge/level triggering
>> or high/low polarity, so it looks to me as if the comment is at
>> least misleading too.
> 
> bit 8 and bit 9 are related to the PPI configuration. We need to know if 
> the interrupt is level/edge trigger active low/high to configure 
> correctly the interrupt controller.
> 
> What about renaming "interrupt" to "PPI" to make clear is related to the 
> PPI?

Maybe. What has confused me more was the "event-channel
interrupt" part in the comment.

Jan


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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-04 21:19   ` Konrad Rzeszutek Wilk
@ 2016-03-16 16:34     ` Julien Grall
  2016-03-16 17:49       ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 63+ messages in thread
From: Julien Grall @ 2016-03-16 16:34 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Shannon Zhao
  Cc: hangaohuai, stefano.stabellini, shannon.zhao, Jan Beulich, xen-devel

Hi Konrad,

On 04/03/2016 21:19, Konrad Rzeszutek Wilk wrote:
> Anyhow what I am wondering if there are some semantincs when it comes to PPI
> and it being able to 'mask' an vector until it exits or such? If so
> you should document that.

I'm not sure to understand what you are asking.

Xen takes advantage of the virtual interrupt controller to notify the 
guest of new pending event channels.

The life cycle (mask/eoi) of the interrupt associated to the 
notification is handled by the interrupt controller. The interrupt can't 
be re-entrant but we may receive spurious notification.

AFAIK, there is no specific semantics. Stefano, can you confirm it?

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-16 16:34     ` Julien Grall
@ 2016-03-16 17:49       ` Konrad Rzeszutek Wilk
  2016-03-24 12:24         ` Stefano Stabellini
  0 siblings, 1 reply; 63+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-03-16 17:49 UTC (permalink / raw)
  To: Julien Grall
  Cc: hangaohuai, xen-devel, stefano.stabellini, shannon.zhao,
	Jan Beulich, Shannon Zhao

On Wed, Mar 16, 2016 at 04:34:19PM +0000, Julien Grall wrote:
> Hi Konrad,
> 
> On 04/03/2016 21:19, Konrad Rzeszutek Wilk wrote:
> >Anyhow what I am wondering if there are some semantincs when it comes to PPI
> >and it being able to 'mask' an vector until it exits or such? If so
> >you should document that.
> 
> I'm not sure to understand what you are asking.
> 
> Xen takes advantage of the virtual interrupt controller to notify the guest
> of new pending event channels.
> 
> The life cycle (mask/eoi) of the interrupt associated to the notification is
> handled by the interrupt controller. The interrupt can't be re-entrant but
> we may receive spurious notification.

Excellent. That is what I wanted to know. It would be good to make
sure that is mentioned in the header file.

> 
> AFAIK, there is no specific semantics. Stefano, can you confirm it?

Because on x86 the vector callback bypasses the interrupt controller and
injects the vector using the mechanism that is usually used for exceptions
(or emulating software interrupts).

> 
> Regards,
> 
> -- 
> Julien Grall

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ
  2016-03-16 17:49       ` Konrad Rzeszutek Wilk
@ 2016-03-24 12:24         ` Stefano Stabellini
  0 siblings, 0 replies; 63+ messages in thread
From: Stefano Stabellini @ 2016-03-24 12:24 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: hangaohuai, xen-devel, Julien Grall, stefano.stabellini,
	shannon.zhao, Jan Beulich, Shannon Zhao

On Wed, 16 Mar 2016, Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 16, 2016 at 04:34:19PM +0000, Julien Grall wrote:
> > Hi Konrad,
> > 
> > On 04/03/2016 21:19, Konrad Rzeszutek Wilk wrote:
> > >Anyhow what I am wondering if there are some semantincs when it comes to PPI
> > >and it being able to 'mask' an vector until it exits or such? If so
> > >you should document that.
> > 
> > I'm not sure to understand what you are asking.
> > 
> > Xen takes advantage of the virtual interrupt controller to notify the guest
> > of new pending event channels.
> > 
> > The life cycle (mask/eoi) of the interrupt associated to the notification is
> > handled by the interrupt controller. The interrupt can't be re-entrant but
> > we may receive spurious notification.
> 
> Excellent. That is what I wanted to know. It would be good to make
> sure that is mentioned in the header file.
> 
> > 
> > AFAIK, there is no specific semantics. Stefano, can you confirm it?

Yes, that's right.


> Because on x86 the vector callback bypasses the interrupt controller and
> injects the vector using the mechanism that is usually used for exceptions
> (or emulating software interrupts).

Right, that's the big difference.

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2016-03-24 12:24 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-04  6:15 [PATCH v5 00/22] Prepare UEFI and ACPI tables for Dom0 on ARM64 Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 01/22] arm/acpi: Estimate memory required for acpi/efi tables Shannon Zhao
2016-03-04 10:09   ` Jan Beulich
2016-03-04 10:55     ` Stefano Stabellini
2016-03-04 15:03       ` Shannon Zhao
2016-03-04 15:23         ` Stefano Stabellini
2016-03-04 15:52           ` Shannon Zhao
2016-03-04 15:59             ` Stefano Stabellini
2016-03-04 15:39         ` Jan Beulich
2016-03-04  6:15 ` [PATCH v5 02/22] arm/acpi: Add a helper function to get the acpi table offset Shannon Zhao
2016-03-04 10:59   ` Stefano Stabellini
2016-03-04 15:12     ` Shannon Zhao
2016-03-04 15:31       ` Stefano Stabellini
2016-03-04 15:53         ` Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 03/22] arm/acpi: Prepare FADT table for Dom0 Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 04/22] arm/gic: Add a new callback for creating MADT " Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 05/22] arm/acpi: Prepare " Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 06/22] arm/acpi: Prepare STAO " Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 07/22] arm/acpi: Prepare XSDT " Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 08/22] arm/acpi: Prepare RSDP " Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 09/22] arm/p2m: Add helper functions to map memory regions Shannon Zhao
2016-03-04 10:51   ` Stefano Stabellini
2016-03-04  6:15 ` [PATCH v5 10/22] arm/acpi: Map all other tables for Dom0 Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 11/22] arm/acpi: Prepare EFI system table " Shannon Zhao
2016-03-04 11:02   ` Stefano Stabellini
2016-03-04  6:15 ` [PATCH v5 12/22] arm/acpi: Prepare EFI memory descriptor " Shannon Zhao
2016-03-04 11:13   ` Stefano Stabellini
2016-03-16  8:59     ` Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0 Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 14/22] arm/acpi: Create min DT stub for Dom0 Shannon Zhao
2016-03-04 11:17   ` Stefano Stabellini
2016-03-04  6:15 ` [PATCH v5 15/22] arm/acpi: Permit access all Xen unused SPIs " Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 16/22] arm/acpi: Configure SPI interrupt type and route to Dom0 dynamically Shannon Zhao
2016-03-04 11:26   ` Stefano Stabellini
2016-03-04 15:16     ` Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 17/22] arm/gic: Add a new callback to deny Dom0 access to GIC regions Shannon Zhao
2016-03-04 11:59   ` Stefano Stabellini
2016-03-04  6:15 ` [PATCH v5 18/22] arm/acpi: Permit MMIO access of Xen unused devices for Dom0 Shannon Zhao
2016-03-04 12:01   ` Stefano Stabellini
2016-03-04  6:15 ` [PATCH v5 19/22] hvm/params: Add a new delivery type for event-channel in HVM_PARAM_CALLBACK_IRQ Shannon Zhao
2016-03-04 10:16   ` Jan Beulich
2016-03-04 12:09     ` Stefano Stabellini
2016-03-04 12:20       ` Jan Beulich
2016-03-04 12:26         ` Stefano Stabellini
2016-03-16 15:03     ` Julien Grall
2016-03-16 15:10       ` Jan Beulich
2016-03-04 21:19   ` Konrad Rzeszutek Wilk
2016-03-16 16:34     ` Julien Grall
2016-03-16 17:49       ` Konrad Rzeszutek Wilk
2016-03-24 12:24         ` Stefano Stabellini
2016-03-04  6:15 ` [PATCH v5 20/22] xen/acpi: Fix event-channel interrupt when booting with ACPI Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 21/22] xen/arm: Add a hypercall for device mmio mapping Shannon Zhao
2016-03-04 10:29   ` Jan Beulich
2016-03-04 11:00     ` Roger Pau Monné
2016-03-04 11:11       ` Jan Beulich
2016-03-04 11:37         ` Stefano Stabellini
2016-03-16  9:48     ` Shannon Zhao
2016-03-16 10:04       ` Jan Beulich
2016-03-16 11:22         ` Shannon Zhao
2016-03-04  6:15 ` [PATCH v5 22/22] xen/arm64: Add ACPI support Shannon Zhao
2016-03-04 10:33   ` Jan Beulich
2016-03-04 11:45     ` Stefano Stabellini
2016-03-04 11:45   ` Stefano Stabellini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).