[PATCH v1 0/7] Make building xSplice patches easier

[PATCH v1 0/7] Make building xSplice patches easier

[Ross Lagerwall]

Here is a set of changes to make building xSplice patches easier.
Tested to boot on x86.
Compile-tested on arm.

This is probably too late to make it into 4.7, but hey, if someone wants
to put it in I've CC'd Wei.

Ross Lagerwall (7):
  lib: Add a generic implementation of current_text_addr()
  sched: Remove dependency on __LINE__ for release builds
  mm: Use statically defined locking order
  page-alloc: Remove dependency on __LINE__ for release builds
  iommu: Remove dependency on __LINE__ for release builds
  acpi: Remove dependency on __LINE__ for release builds
  Rename sections for compatibility with -ffunction-sections
    -fdata-sections

 xen/arch/arm/xen.lds.S              | 14 +++++------
 xen/arch/x86/boot/x86_64.S          |  2 +-
 xen/arch/x86/hvm/hvm.c              |  2 +-
 xen/arch/x86/mm.c                   |  4 ++--
 xen/arch/x86/mm/mm-locks.h          | 29 +++++++++++++++--------
 xen/arch/x86/setup.c                |  2 +-
 xen/arch/x86/x86_64/kexec_reloc.S   |  2 +-
 xen/arch/x86/xen.lds.S              | 18 +++++++--------
 xen/common/lib.c                    | 12 ++++++++++
 xen/common/page_alloc.c             |  8 +++++++
 xen/drivers/acpi/utilities/utmisc.c | 46 +++++++++++++++++++++++++++++++++++++
 xen/drivers/passthrough/vtd/dmar.h  | 12 ++++++++--
 xen/include/acpi/acmacros.h         |  4 ++++
 xen/include/acpi/acutils.h          | 19 +++++++++++++++
 xen/include/asm-arm/cache.h         |  2 +-
 xen/include/asm-arm/percpu.h        |  2 +-
 xen/include/asm-x86/cache.h         |  2 +-
 xen/include/asm-x86/percpu.h        |  2 +-
 xen/include/asm-x86/processor.h     | 10 --------
 xen/include/xen/lib.h               |  2 ++
 xen/include/xen/sched-if.h          |  2 +-
 xen/include/xen/sched.h             | 14 +++++++++++
 22 files changed, 162 insertions(+), 48 deletions(-)

-- 
2.4.3


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

[PATCH v1 1/7] lib: Add a generic implementation of current_text_addr()

[Ross Lagerwall]

Remove the unused x86 implementation.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/common/lib.c                | 12 ++++++++++++
 xen/include/asm-x86/processor.h | 10 ----------
 xen/include/xen/lib.h           |  2 ++
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/xen/common/lib.c b/xen/common/lib.c
index ae0bbb3..fdcead8 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -499,6 +499,18 @@ void __init init_constructors(void)
 }
 
 /*
+ * The GCC docs state that the function must be marked noinline to have the
+ * expected result:
+ * "When inlining the expected behavior is that the function returns the
+ * address of the function that is returned to. To work around this behavior
+ * use the noinline function attribute."
+ */
+noinline void *current_text_addr(void)
+{
+    return __builtin_return_address(0);
+}
+
+/*
  * Local variables:
  * mode: C
  * c-file-style: "BSD"
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 37ac9e4..bca1267 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -160,16 +160,6 @@
 struct domain;
 struct vcpu;
 
-/*
- * Default implementation of macro that returns current
- * instruction pointer ("program counter").
- */
-#define current_text_addr() ({                      \
-    void *pc;                                       \
-    asm ( "leaq 1f(%%rip),%0\n1:" : "=r" (pc) );    \
-    pc;                                             \
-})
-
 struct x86_cpu_id {
     uint16_t vendor;
     uint16_t family;
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 1c652bb..25e01f1 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -152,4 +152,6 @@ void dump_execstate(struct cpu_user_regs *);
 
 void init_constructors(void);
 
+void *current_text_addr(void);
+
 #endif /* __LIB_H__ */
-- 
2.4.3


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

[PATCH v1 2/7] sched: Remove dependency on __LINE__ for release builds

[Ross Lagerwall]

When using xsplice, use of __LINE__ can generate spurious changes in
functions due to embedded line numbers.  For release builds, remove the
use of these line numbers in domain_crash*() and print the current text
address instead.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/include/xen/sched.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index fe15e9c..b282671 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -611,20 +611,34 @@ void vcpu_end_shutdown_deferral(struct vcpu *v);
  * from any processor.
  */
 void __domain_crash(struct domain *d);
+#ifdef NDEBUG
+#define domain_crash(d) do {                                              \
+    printk("domain_crash called from %p\n", current_text_addr());         \
+    __domain_crash(d);                                                    \
+} while (0)
+#else
 #define domain_crash(d) do {                                              \
     printk("domain_crash called from %s:%d\n", __FILE__, __LINE__);       \
     __domain_crash(d);                                                    \
 } while (0)
+#endif
 
 /*
  * Mark current domain as crashed and synchronously deschedule from the local
  * processor. This function never returns.
  */
 void noreturn __domain_crash_synchronous(void);
+#ifdef NDEBUG
+#define domain_crash_synchronous() do {                                   \
+    printk("domain_crash_sync called from %p\n", current_text_addr());    \
+    __domain_crash_synchronous();                                         \
+} while (0)
+#else
 #define domain_crash_synchronous() do {                                   \
     printk("domain_crash_sync called from %s:%d\n", __FILE__, __LINE__);  \
     __domain_crash_synchronous();                                         \
 } while (0)
+#endif
 
 /*
  * Called from assembly code, with an optional address to help indicate why
-- 
2.4.3


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

[PATCH v1 3/7] mm: Use statically defined locking order

[Ross Lagerwall]

Instead of using a locking order based on line numbers which doesn't
play nicely with xSplice, statically define the locking order.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/mm/mm-locks.h | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/mm/mm-locks.h b/xen/arch/x86/mm/mm-locks.h
index 086c8bb..da7f52d 100644
--- a/xen/arch/x86/mm/mm-locks.h
+++ b/xen/arch/x86/mm/mm-locks.h
@@ -46,8 +46,10 @@ static inline int mm_locked_by_me(mm_lock_t *l)
     return (l->lock.recurse_cpu == current->processor);
 }
 
-/* If you see this crash, the numbers printed are lines in this file 
- * where the offending locks are declared. */
+/*
+ * If you see this crash, the numbers printed are order levels defined
+ * in this file.
+ */
 #define __check_lock_level(l)                           \
 do {                                                    \
     if ( unlikely(__get_lock_level() > (l)) )           \
@@ -152,12 +154,12 @@ static inline void mm_read_unlock(mm_rwlock_t *l)
 /* This wrapper uses the line number to express the locking order below */
 #define declare_mm_lock(name)                                                 \
     static inline void mm_lock_##name(mm_lock_t *l, const char *func, int rec)\
-    { _mm_lock(l, func, __LINE__, rec); }
+    { _mm_lock(l, func, MM_LOCK_ORDER_##name, rec); }
 #define declare_mm_rwlock(name)                                               \
     static inline void mm_write_lock_##name(mm_rwlock_t *l, const char *func) \
-    { _mm_write_lock(l, func, __LINE__); }                                    \
+    { _mm_write_lock(l, func, MM_LOCK_ORDER_##name); }                                    \
     static inline void mm_read_lock_##name(mm_rwlock_t *l)                    \
-    { _mm_read_lock(l, __LINE__); }
+    { _mm_read_lock(l, MM_LOCK_ORDER_##name); }
 /* These capture the name of the calling function */
 #define mm_lock(name, l) mm_lock_##name(l, __func__, 0)
 #define mm_lock_recursive(name, l) mm_lock_##name(l, __func__, 1)
@@ -169,10 +171,10 @@ static inline void mm_read_unlock(mm_rwlock_t *l)
  * to ordering constraints. */
 #define declare_mm_order_constraint(name)                                   \
     static inline void mm_enforce_order_lock_pre_##name(void)               \
-    { _mm_enforce_order_lock_pre(__LINE__); }                               \
+    { _mm_enforce_order_lock_pre(MM_LOCK_ORDER_##name); }                               \
     static inline void mm_enforce_order_lock_post_##name(                   \
                         int *unlock_level, unsigned short *recurse_count)   \
-    { _mm_enforce_order_lock_post(__LINE__, unlock_level, recurse_count); } \
+    { _mm_enforce_order_lock_post(MM_LOCK_ORDER_##name, unlock_level, recurse_count); } \
 
 static inline void mm_unlock(mm_lock_t *l)
 {
@@ -201,11 +203,20 @@ static inline void mm_enforce_order_unlock(int unlock_level,
 
 /************************************************************************
  *                                                                      *
- * To avoid deadlocks, these locks _MUST_ be taken in the order they're *
- * declared in this file.  The locking functions will enforce this.     *
+ * To avoid deadlocks, these locks _MUST_ be taken in the order listed  *
+ * below.  The locking functions will enforce this.                     *
  *                                                                      *
  ************************************************************************/
 
+#define MM_LOCK_ORDER_nestedp2m              10000
+#define MM_LOCK_ORDER_p2m                    20000
+#define MM_LOCK_ORDER_altp2mlist             30000
+#define MM_LOCK_ORDER_altp2m                 40000
+#define MM_LOCK_ORDER_per_page_sharing       50000
+#define MM_LOCK_ORDER_pod                    60000
+#define MM_LOCK_ORDER_page_alloc             70000
+#define MM_LOCK_ORDER_paging                 80000
+
 /* Nested P2M lock (per-domain)
  *
  * A per-domain lock that protects the mapping from nested-CR3 to
-- 
2.4.3


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

[PATCH v1 4/7] page-alloc: Remove dependency on __LINE__ for release builds

[Ross Lagerwall]

When using xsplice, use of __LINE__ can generate spurious changes in
functions due to embedded line numbers.  For release builds, remove the
use of these line numbers in BOOT_BUG_ON() and print the current text
address instead.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/common/page_alloc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 98e30e5..8355894 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -206,11 +206,19 @@ struct scrub_region {
 static struct scrub_region __initdata region[MAX_NUMNODES];
 static unsigned long __initdata chunk_size;
 
+#ifdef NDEBUG
+static void __init boot_bug(void *addr)
+{
+    panic("Boot BUG at %p", addr);
+}
+#define BOOT_BUG_ON(p) if ( p ) boot_bug(current_text_addr());
+#else
 static void __init boot_bug(int line)
 {
     panic("Boot BUG at %s:%d", __FILE__, line);
 }
 #define BOOT_BUG_ON(p) if ( p ) boot_bug(__LINE__);
+#endif
 
 static void __init bootmem_region_add(unsigned long s, unsigned long e)
 {
-- 
2.4.3


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

[PATCH v1 5/7] iommu: Remove dependency on __LINE__ for release builds

[Ross Lagerwall]

When using xsplice, use of __LINE__ can generate spurious changes in
functions due to embedded line numbers.  For release builds, remove the
use of these line numbers in IOMMU_WAIT_OP() and print the current text
address instead.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/drivers/passthrough/vtd/dmar.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.h b/xen/drivers/passthrough/vtd/dmar.h
index 729b603..3a7bfe0 100644
--- a/xen/drivers/passthrough/vtd/dmar.h
+++ b/xen/drivers/passthrough/vtd/dmar.h
@@ -108,6 +108,15 @@ struct acpi_atsr_unit *acpi_find_matched_atsr_unit(const struct pci_dev *);
 
 #define DMAR_OPERATION_TIMEOUT MILLISECS(1000)
 
+#ifdef NDEBUG
+#define IOMMU_WAIT_OP_PANIC \
+    panic("%p: DMAR hardware is malfunctional", current_text_addr());
+#else
+#define IOMMU_WAIT_OP_PANIC \
+    panic("%s:%d:%s: DMAR hardware is malfunctional",  \
+          __FILE__, __LINE__, __func__);
+#endif
+
 #define IOMMU_WAIT_OP(iommu, offset, op, cond, sts) \
 do {                                                \
     s_time_t start_time = NOW();                    \
@@ -117,8 +126,7 @@ do {                                                \
             break;                                  \
         if ( NOW() > start_time + DMAR_OPERATION_TIMEOUT ) {    \
             if ( !kexecing )                                    \
-                panic("%s:%d:%s: DMAR hardware is malfunctional",\
-                      __FILE__, __LINE__, __func__);            \
+                IOMMU_WAIT_OP_PANIC                             \
             else                                                \
                 break;                                          \
         }                                                       \
-- 
2.4.3


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

[PATCH v1 6/7] acpi: Remove dependency on __LINE__ for release builds

[Ross Lagerwall]

When using xsplice, use of __LINE__ can generate spurious changes in
functions due to embedded line numbers.  For release builds, remove the
use of these line numbers in the ACPI code and print the current text
address instead.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/drivers/acpi/utilities/utmisc.c | 46 +++++++++++++++++++++++++++++++++++++
 xen/include/acpi/acmacros.h         |  4 ++++
 xen/include/acpi/acutils.h          | 19 +++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/xen/drivers/acpi/utilities/utmisc.c b/xen/drivers/acpi/utilities/utmisc.c
index b3e3012..8ace116 100644
--- a/xen/drivers/acpi/utilities/utmisc.c
+++ b/xen/drivers/acpi/utilities/utmisc.c
@@ -134,6 +134,51 @@ const char *__init acpi_ut_validate_exception(acpi_status status)
  *
  ******************************************************************************/
 
+#ifdef NDEBUG
+void ACPI_INTERNAL_VAR_XFACE __init
+acpi_ut_error(const char *module_name, void *addr, char *format, ...)
+{
+	va_list args;
+
+	acpi_os_printf("ACPI Error (%s-%p): ", module_name, addr);
+
+	va_start(args, format);
+	acpi_os_vprintf(format, args);
+	acpi_os_printf(" [%X]\n", ACPI_CA_VERSION);
+	va_end(args);
+}
+
+void ACPI_INTERNAL_VAR_XFACE __init
+acpi_ut_warning(const char *module_name, void *addr, char *format, ...)
+{
+	va_list args;
+
+	acpi_os_printf("ACPI Warning (%s-%p): ", module_name, addr);
+
+	va_start(args, format);
+	acpi_os_vprintf(format, args);
+	acpi_os_printf(" [%X]\n", ACPI_CA_VERSION);
+	va_end(args);
+	va_end(args);
+}
+
+void ACPI_INTERNAL_VAR_XFACE __init
+acpi_ut_info(const char *module_name, void *addr, char *format, ...)
+{
+	va_list args;
+
+	/*
+	 * Removed module_name, line_number, and acpica version, not needed
+	 * for info output
+	 */
+	acpi_os_printf("ACPI: ");
+
+	va_start(args, format);
+	acpi_os_vprintf(format, args);
+	acpi_os_printf("\n");
+	va_end(args);
+}
+#else
 void ACPI_INTERNAL_VAR_XFACE __init
 acpi_ut_error(const char *module_name, u32 line_number, char *format, ...)
 {
@@ -177,3 +222,4 @@ acpi_ut_info(const char *module_name, u32 line_number, char *format, ...)
 	acpi_os_printf("\n");
 	va_end(args);
 }
+#endif
diff --git a/xen/include/acpi/acmacros.h b/xen/include/acpi/acmacros.h
index 6765535..d8a2a14 100644
--- a/xen/include/acpi/acmacros.h
+++ b/xen/include/acpi/acmacros.h
@@ -422,7 +422,11 @@
  * Ascii error messages can be configured out
  */
 #ifndef ACPI_NO_ERROR_MESSAGES
+#ifdef NDEBUG
+#define AE_INFO                         _acpi_module_name, current_text_addr()
+#else
 #define AE_INFO                         _acpi_module_name, __LINE__
+#endif
 
 /*
  * Error reporting. Callers module and line number are inserted by AE_INFO,
diff --git a/xen/include/acpi/acutils.h b/xen/include/acpi/acutils.h
index b1b0df7..708256d 100644
--- a/xen/include/acpi/acutils.h
+++ b/xen/include/acpi/acutils.h
@@ -174,6 +174,24 @@ acpi_ut_debug_print_raw(u32 requested_debug_level,
 			u32 component_id,
 			char *format, ...) ACPI_PRINTF_LIKE(6);
 
+#ifdef NDEBUG
+void ACPI_INTERNAL_VAR_XFACE
+acpi_ut_error(const char *module_name,
+	      void *addr, char *format, ...) ACPI_PRINTF_LIKE(3);
+
+void ACPI_INTERNAL_VAR_XFACE
+acpi_ut_exception(const char *module_name,
+		  void *addr,
+		  acpi_status status, char *format, ...) ACPI_PRINTF_LIKE(4);
+
+void ACPI_INTERNAL_VAR_XFACE
+acpi_ut_warning(const char *module_name,
+		void *addr, char *format, ...) ACPI_PRINTF_LIKE(3);
+
+void ACPI_INTERNAL_VAR_XFACE
+acpi_ut_info(const char *module_name,
+	     void *addr, char *format, ...) ACPI_PRINTF_LIKE(3);
+#else
 void ACPI_INTERNAL_VAR_XFACE
 acpi_ut_error(const char *module_name,
 	      u32 line_number, char *format, ...) ACPI_PRINTF_LIKE(3);
@@ -190,6 +208,7 @@ acpi_ut_warning(const char *module_name,
 void ACPI_INTERNAL_VAR_XFACE
 acpi_ut_info(const char *module_name,
 	     u32 line_number, char *format, ...) ACPI_PRINTF_LIKE(3);
+#endif
 
 /*
  * utmisc
-- 
2.4.3


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

[PATCH v1 7/7] Rename sections for compatibility with -ffunction-sections -fdata-sections

[Ross Lagerwall]

When building with -ffunction-sections -fdata-sections, it will generate
section names like .text.show_handlers and .data.payload_list. These
sections are in the same namespace as the special sections that Xen
uses, such as .text.kexec and .data.schedulers. To prevent conflicts,
prefix Xen's special sections with an extra period.

The idea for this was taken from a similar patch series applied to the
Linux kernel by the kSplice folks.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/arm/xen.lds.S            | 14 +++++++-------
 xen/arch/x86/boot/x86_64.S        |  2 +-
 xen/arch/x86/hvm/hvm.c            |  2 +-
 xen/arch/x86/mm.c                 |  4 ++--
 xen/arch/x86/setup.c              |  2 +-
 xen/arch/x86/x86_64/kexec_reloc.S |  2 +-
 xen/arch/x86/xen.lds.S            | 18 +++++++++---------
 xen/include/asm-arm/cache.h       |  2 +-
 xen/include/asm-arm/percpu.h      |  2 +-
 xen/include/asm-x86/cache.h       |  2 +-
 xen/include/asm-x86/percpu.h      |  2 +-
 xen/include/xen/sched-if.h        |  2 +-
 12 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 1f010bd..ec6c389 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -74,11 +74,11 @@ SECTIONS
 
   .data : {                    /* Data */
        . = ALIGN(PAGE_SIZE);
-       *(.data.page_aligned)
+       *(.data..page_aligned)
        *(.data)
        . = ALIGN(8);
        __start_schedulers_array = .;
-       *(.data.schedulers)
+       *(.data..schedulers)
        __end_schedulers_array = .;
        *(.data.rel)
        *(.data.rel.*)
@@ -97,7 +97,7 @@ SECTIONS
        *(.ex_table.pre)
        __stop___pre_ex_table = .;
 
-       *(.data.read_mostly)
+       *(.data..read_mostly)
        *(.data.rel.ro)
        *(.data.rel.ro.*)
   } :text
@@ -173,15 +173,15 @@ SECTIONS
 
   .bss : {                     /* BSS */
        __bss_start = .;
-       *(.bss.stack_aligned)
+       *(.bss..stack_aligned)
        . = ALIGN(PAGE_SIZE);
-       *(.bss.page_aligned)
+       *(.bss..page_aligned)
        *(.bss)
        . = ALIGN(SMP_CACHE_BYTES);
        __per_cpu_start = .;
-       *(.bss.percpu)
+       *(.bss..percpu)
        . = ALIGN(SMP_CACHE_BYTES);
-       *(.bss.percpu.read_mostly)
+       *(.bss..percpu.read_mostly)
        . = ALIGN(SMP_CACHE_BYTES);
        __per_cpu_data_end = .;
        __bss_end = .;
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 9ab9231..12b0ba8 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -51,7 +51,7 @@ GLOBAL(gdt_descr)
 GLOBAL(stack_start)
         .quad   cpu0_stack
 
-        .section .data.page_aligned, "aw", @progbits
+        .section .data..page_aligned, "aw", @progbits
         .align PAGE_SIZE, 0
 GLOBAL(boot_cpu_gdt_table)
         .quad 0x0000000000000000     /* unused */
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 82e2ed1..3eb2369 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -88,7 +88,7 @@ struct hvm_function_table hvm_funcs __read_mostly;
  * the hardware domain which needs a more permissive one.
  */
 #define HVM_IOBITMAP_SIZE (3 * PAGE_SIZE)
-unsigned long __section(".bss.page_aligned")
+unsigned long __section(".bss..page_aligned")
     hvm_io_bitmap[HVM_IOBITMAP_SIZE / BYTES_PER_LONG];
 
 /* Xen command-line option to enable HAP */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 2bb920b..5b59f7d 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -125,7 +125,7 @@
 #include <asm/pci.h>
 
 /* Mapping of the fixmap space needed early. */
-l1_pgentry_t __section(".bss.page_aligned") l1_fixmap[L1_PAGETABLE_ENTRIES];
+l1_pgentry_t __section(".bss..page_aligned") l1_fixmap[L1_PAGETABLE_ENTRIES];
 
 #define MEM_LOG(_f, _a...) gdprintk(XENLOG_WARNING , _f "\n" , ## _a)
 
@@ -589,7 +589,7 @@ static inline void guest_get_eff_kern_l1e(struct vcpu *v, unsigned long addr,
     TOGGLE_MODE();
 }
 
-const char __section(".bss.page_aligned.const") zero_page[PAGE_SIZE];
+const char __section(".bss..page_aligned.const") zero_page[PAGE_SIZE];
 
 static void invalidate_shadow_ldt(struct vcpu *v, int flush)
 {
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5029568..7d30945 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -103,7 +103,7 @@ unsigned long __read_mostly xen_virt_end;
 
 DEFINE_PER_CPU(struct tss_struct, init_tss);
 
-char __section(".bss.stack_aligned") cpu0_stack[STACK_SIZE];
+char __section(".bss..stack_aligned") cpu0_stack[STACK_SIZE];
 
 struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
 
diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
index 85ab602..8ce87ec 100644
--- a/xen/arch/x86/x86_64/kexec_reloc.S
+++ b/xen/arch/x86/x86_64/kexec_reloc.S
@@ -18,7 +18,7 @@
 #include <asm/page.h>
 #include <asm/machine_kexec.h>
 
-        .section .text.kexec, "ax", @progbits
+        .section .text..kexec, "ax", @progbits
         .align PAGE_SIZE
         .code64
 
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index b14bcd2..aa9467d 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -52,7 +52,7 @@ SECTIONS
        *(.text.cold)
        *(.text.unlikely)
        *(.fixup)
-       *(.text.kexec)
+       *(.text..kexec)
        *(.gnu.warning)
        _etext = .;             /* End of text section */
   } :text = 0x9090
@@ -207,11 +207,11 @@ SECTIONS
 
   __2M_rwdata_start = .;       /* Start of 2M superpages, mapped RW. */
   . = ALIGN(SMP_CACHE_BYTES);
-  .data.read_mostly : {
-       *(.data.read_mostly)
+  .data..read_mostly : {
+       *(.data..read_mostly)
        . = ALIGN(8);
        __start_schedulers_array = .;
-       *(.data.schedulers)
+       *(.data..schedulers)
        __end_schedulers_array = .;
        *(.data.rel.ro)
        *(.data.rel.ro.*)
@@ -219,7 +219,7 @@ SECTIONS
 
   .data : {                    /* Data */
        . = ALIGN(PAGE_SIZE);
-       *(.data.page_aligned)
+       *(.data..page_aligned)
        *(.data)
        *(.data.rel)
        *(.data.rel.*)
@@ -229,15 +229,15 @@ SECTIONS
   .bss : {                     /* BSS */
        . = ALIGN(STACK_SIZE);
        __bss_start = .;
-       *(.bss.stack_aligned)
+       *(.bss..stack_aligned)
        . = ALIGN(PAGE_SIZE);
-       *(.bss.page_aligned*)
+       *(.bss..page_aligned*)
        *(.bss)
        . = ALIGN(SMP_CACHE_BYTES);
        __per_cpu_start = .;
-       *(.bss.percpu)
+       *(.bss..percpu)
        . = ALIGN(SMP_CACHE_BYTES);
-       *(.bss.percpu.read_mostly)
+       *(.bss..percpu.read_mostly)
        . = ALIGN(SMP_CACHE_BYTES);
        __per_cpu_data_end = .;
        __bss_end = .;
diff --git a/xen/include/asm-arm/cache.h b/xen/include/asm-arm/cache.h
index 2de6564..7723b06 100644
--- a/xen/include/asm-arm/cache.h
+++ b/xen/include/asm-arm/cache.h
@@ -7,7 +7,7 @@
 #define L1_CACHE_SHIFT  (CONFIG_ARM_L1_CACHE_SHIFT)
 #define L1_CACHE_BYTES  (1 << L1_CACHE_SHIFT)
 
-#define __read_mostly __section(".data.read_mostly")
+#define __read_mostly __section(".data..read_mostly")
 
 #endif
 /*
diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h
index 7968532..f4d9628 100644
--- a/xen/include/asm-arm/percpu.h
+++ b/xen/include/asm-arm/percpu.h
@@ -19,7 +19,7 @@ void percpu_init_areas(void);
 
 /* Separate out the type, so (int[3], foo) works. */
 #define __DEFINE_PER_CPU(type, name, suffix)                    \
-    __section(".bss.percpu" #suffix)                            \
+    __section(".bss..percpu" #suffix)                            \
     __typeof__(type) per_cpu_##name
 
 #define per_cpu(var, cpu)  \
diff --git a/xen/include/asm-x86/cache.h b/xen/include/asm-x86/cache.h
index f4a08e7..5aedb85 100644
--- a/xen/include/asm-x86/cache.h
+++ b/xen/include/asm-x86/cache.h
@@ -10,6 +10,6 @@
 #define L1_CACHE_SHIFT	(CONFIG_X86_L1_CACHE_SHIFT)
 #define L1_CACHE_BYTES	(1 << L1_CACHE_SHIFT)
 
-#define __read_mostly __section(".data.read_mostly")
+#define __read_mostly __section(".data..read_mostly")
 
 #endif
diff --git a/xen/include/asm-x86/percpu.h b/xen/include/asm-x86/percpu.h
index 51562b9..5d442c3 100644
--- a/xen/include/asm-x86/percpu.h
+++ b/xen/include/asm-x86/percpu.h
@@ -9,7 +9,7 @@ void percpu_init_areas(void);
 
 /* Separate out the type, so (int[3], foo) works. */
 #define __DEFINE_PER_CPU(type, name, suffix)                    \
-    __section(".bss.percpu" #suffix)                            \
+    __section(".bss..percpu" #suffix)                            \
     __typeof__(type) per_cpu_##name
 
 /* var is in discarded region: offset to particular copy we want */
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index bc0e794..78c6462 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -175,7 +175,7 @@ struct scheduler {
 };
 
 #define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \
-  __used_section(".data.schedulers") = &x;
+  __used_section(".data..schedulers") = &x;
 
 struct cpupool
 {
-- 
2.4.3


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

Re: [PATCH v1 0/7] Make building xSplice patches easier

[Wei Liu]

On Fri, May 06, 2016 at 04:48:01PM +0100, Ross Lagerwall wrote:
> Here is a set of changes to make building xSplice patches easier.
> Tested to boot on x86.
> Compile-tested on arm.
> 
> This is probably too late to make it into 4.7, but hey, if someone wants
> to put it in I've CC'd Wei.
> 

Unfortunately I think this is too late for 4.7.

Wei. 

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

Re: [PATCH v1 1/7] lib: Add a generic implementation of current_text_addr()

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1496 bytes --]

[adding Roger]

On Fri, 2016-05-06 at 16:48 +0100, Ross Lagerwall wrote:
> Remove the unused x86 implementation.
> 
Hey, this is actually very useful, for another thing I'm working on...
Thanks! :-P

> --- a/xen/common/lib.c
> +++ b/xen/common/lib.c
> @@ -499,6 +499,18 @@ void __init init_constructors(void)
>  }
>  
>  /*
> + * The GCC docs state that the function must be marked noinline to
> have the
> + * expected result:
> + * "When inlining the expected behavior is that the function returns
> the
> + * address of the function that is returned to. To work around this
> behavior
> + * use the noinline function attribute."
> + */
> +noinline void *current_text_addr(void)
> +{
> +    return __builtin_return_address(0);
> +}
> +
Since we started to care about clang, what's the situation about it?
From what I read in the links below, it looks like it's similar, and
that noinline would actually be ok there too... in which case, it's
probably worth making the comment either reference both docs, or be a
bit more abstract/generic.

http://lists.llvm.org/pipermail/llvm-dev/2015-December/092893.html
http://llvm.org/docs/LangRef.html#llvm-returnaddress-intrinsic

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

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

Re: [PATCH v1 3/7] mm: Use statically defined locking order

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1288 bytes --]

On Fri, 2016-05-06 at 16:48 +0100, Ross Lagerwall wrote:
> Instead of using a locking order based on line numbers which doesn't
> play nicely with xSplice, statically define the locking order.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

FWIW, I actually like this...

> --- a/xen/arch/x86/mm/mm-locks.h
> +++ b/xen/arch/x86/mm/mm-locks.h
> @@ -46,8 +46,10 @@ static inline int mm_locked_by_me(mm_lock_t *l)
>      return (l->lock.recurse_cpu == current->processor);
>  }
>  
> -/* If you see this crash, the numbers printed are lines in this
> file 
> - * where the offending locks are declared. */
> +/*
> + * If you see this crash, the numbers printed are order levels
> defined
> + * in this file.
> + */
>
... and, apart from the fat that it server xsplice purposes, I'd say
this is an improvement on its own of the current situation (although I
understand this is a matter of taste). :-)

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

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

Re: [PATCH v1 7/7] Rename sections for compatibility with -ffunction-sections -fdata-sections

[Julien Grall]

Hi Ross,

On 06/05/16 16:48, Ross Lagerwall wrote:
> When building with -ffunction-sections -fdata-sections, it will generate
> section names like .text.show_handlers and .data.payload_list. These
> sections are in the same namespace as the special sections that Xen
> uses, such as .text.kexec and .data.schedulers. To prevent conflicts,
> prefix Xen's special sections with an extra period.
>
> The idea for this was taken from a similar patch series applied to the
> Linux kernel by the kSplice folks.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

For the ARM parts:

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

Regards,

> ---
>   xen/arch/arm/xen.lds.S            | 14 +++++++-------
>   xen/arch/x86/boot/x86_64.S        |  2 +-
>   xen/arch/x86/hvm/hvm.c            |  2 +-
>   xen/arch/x86/mm.c                 |  4 ++--
>   xen/arch/x86/setup.c              |  2 +-
>   xen/arch/x86/x86_64/kexec_reloc.S |  2 +-
>   xen/arch/x86/xen.lds.S            | 18 +++++++++---------
>   xen/include/asm-arm/cache.h       |  2 +-
>   xen/include/asm-arm/percpu.h      |  2 +-
>   xen/include/asm-x86/cache.h       |  2 +-
>   xen/include/asm-x86/percpu.h      |  2 +-
>   xen/include/xen/sched-if.h        |  2 +-
>   12 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 1f010bd..ec6c389 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -74,11 +74,11 @@ SECTIONS
>
>     .data : {                    /* Data */
>          . = ALIGN(PAGE_SIZE);
> -       *(.data.page_aligned)
> +       *(.data..page_aligned)
>          *(.data)
>          . = ALIGN(8);
>          __start_schedulers_array = .;
> -       *(.data.schedulers)
> +       *(.data..schedulers)
>          __end_schedulers_array = .;
>          *(.data.rel)
>          *(.data.rel.*)
> @@ -97,7 +97,7 @@ SECTIONS
>          *(.ex_table.pre)
>          __stop___pre_ex_table = .;
>
> -       *(.data.read_mostly)
> +       *(.data..read_mostly)
>          *(.data.rel.ro)
>          *(.data.rel.ro.*)
>     } :text
> @@ -173,15 +173,15 @@ SECTIONS
>
>     .bss : {                     /* BSS */
>          __bss_start = .;
> -       *(.bss.stack_aligned)
> +       *(.bss..stack_aligned)
>          . = ALIGN(PAGE_SIZE);
> -       *(.bss.page_aligned)
> +       *(.bss..page_aligned)
>          *(.bss)
>          . = ALIGN(SMP_CACHE_BYTES);
>          __per_cpu_start = .;
> -       *(.bss.percpu)
> +       *(.bss..percpu)
>          . = ALIGN(SMP_CACHE_BYTES);
> -       *(.bss.percpu.read_mostly)
> +       *(.bss..percpu.read_mostly)
>          . = ALIGN(SMP_CACHE_BYTES);
>          __per_cpu_data_end = .;
>          __bss_end = .;
> diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
> index 9ab9231..12b0ba8 100644
> --- a/xen/arch/x86/boot/x86_64.S
> +++ b/xen/arch/x86/boot/x86_64.S
> @@ -51,7 +51,7 @@ GLOBAL(gdt_descr)
>   GLOBAL(stack_start)
>           .quad   cpu0_stack
>
> -        .section .data.page_aligned, "aw", @progbits
> +        .section .data..page_aligned, "aw", @progbits
>           .align PAGE_SIZE, 0
>   GLOBAL(boot_cpu_gdt_table)
>           .quad 0x0000000000000000     /* unused */
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 82e2ed1..3eb2369 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -88,7 +88,7 @@ struct hvm_function_table hvm_funcs __read_mostly;
>    * the hardware domain which needs a more permissive one.
>    */
>   #define HVM_IOBITMAP_SIZE (3 * PAGE_SIZE)
> -unsigned long __section(".bss.page_aligned")
> +unsigned long __section(".bss..page_aligned")
>       hvm_io_bitmap[HVM_IOBITMAP_SIZE / BYTES_PER_LONG];
>
>   /* Xen command-line option to enable HAP */
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 2bb920b..5b59f7d 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -125,7 +125,7 @@
>   #include <asm/pci.h>
>
>   /* Mapping of the fixmap space needed early. */
> -l1_pgentry_t __section(".bss.page_aligned") l1_fixmap[L1_PAGETABLE_ENTRIES];
> +l1_pgentry_t __section(".bss..page_aligned") l1_fixmap[L1_PAGETABLE_ENTRIES];
>
>   #define MEM_LOG(_f, _a...) gdprintk(XENLOG_WARNING , _f "\n" , ## _a)
>
> @@ -589,7 +589,7 @@ static inline void guest_get_eff_kern_l1e(struct vcpu *v, unsigned long addr,
>       TOGGLE_MODE();
>   }
>
> -const char __section(".bss.page_aligned.const") zero_page[PAGE_SIZE];
> +const char __section(".bss..page_aligned.const") zero_page[PAGE_SIZE];
>
>   static void invalidate_shadow_ldt(struct vcpu *v, int flush)
>   {
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 5029568..7d30945 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -103,7 +103,7 @@ unsigned long __read_mostly xen_virt_end;
>
>   DEFINE_PER_CPU(struct tss_struct, init_tss);
>
> -char __section(".bss.stack_aligned") cpu0_stack[STACK_SIZE];
> +char __section(".bss..stack_aligned") cpu0_stack[STACK_SIZE];
>
>   struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
>
> diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S
> index 85ab602..8ce87ec 100644
> --- a/xen/arch/x86/x86_64/kexec_reloc.S
> +++ b/xen/arch/x86/x86_64/kexec_reloc.S
> @@ -18,7 +18,7 @@
>   #include <asm/page.h>
>   #include <asm/machine_kexec.h>
>
> -        .section .text.kexec, "ax", @progbits
> +        .section .text..kexec, "ax", @progbits
>           .align PAGE_SIZE
>           .code64
>
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index b14bcd2..aa9467d 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -52,7 +52,7 @@ SECTIONS
>          *(.text.cold)
>          *(.text.unlikely)
>          *(.fixup)
> -       *(.text.kexec)
> +       *(.text..kexec)
>          *(.gnu.warning)
>          _etext = .;             /* End of text section */
>     } :text = 0x9090
> @@ -207,11 +207,11 @@ SECTIONS
>
>     __2M_rwdata_start = .;       /* Start of 2M superpages, mapped RW. */
>     . = ALIGN(SMP_CACHE_BYTES);
> -  .data.read_mostly : {
> -       *(.data.read_mostly)
> +  .data..read_mostly : {
> +       *(.data..read_mostly)
>          . = ALIGN(8);
>          __start_schedulers_array = .;
> -       *(.data.schedulers)
> +       *(.data..schedulers)
>          __end_schedulers_array = .;
>          *(.data.rel.ro)
>          *(.data.rel.ro.*)
> @@ -219,7 +219,7 @@ SECTIONS
>
>     .data : {                    /* Data */
>          . = ALIGN(PAGE_SIZE);
> -       *(.data.page_aligned)
> +       *(.data..page_aligned)
>          *(.data)
>          *(.data.rel)
>          *(.data.rel.*)
> @@ -229,15 +229,15 @@ SECTIONS
>     .bss : {                     /* BSS */
>          . = ALIGN(STACK_SIZE);
>          __bss_start = .;
> -       *(.bss.stack_aligned)
> +       *(.bss..stack_aligned)
>          . = ALIGN(PAGE_SIZE);
> -       *(.bss.page_aligned*)
> +       *(.bss..page_aligned*)
>          *(.bss)
>          . = ALIGN(SMP_CACHE_BYTES);
>          __per_cpu_start = .;
> -       *(.bss.percpu)
> +       *(.bss..percpu)
>          . = ALIGN(SMP_CACHE_BYTES);
> -       *(.bss.percpu.read_mostly)
> +       *(.bss..percpu.read_mostly)
>          . = ALIGN(SMP_CACHE_BYTES);
>          __per_cpu_data_end = .;
>          __bss_end = .;
> diff --git a/xen/include/asm-arm/cache.h b/xen/include/asm-arm/cache.h
> index 2de6564..7723b06 100644
> --- a/xen/include/asm-arm/cache.h
> +++ b/xen/include/asm-arm/cache.h
> @@ -7,7 +7,7 @@
>   #define L1_CACHE_SHIFT  (CONFIG_ARM_L1_CACHE_SHIFT)
>   #define L1_CACHE_BYTES  (1 << L1_CACHE_SHIFT)
>
> -#define __read_mostly __section(".data.read_mostly")
> +#define __read_mostly __section(".data..read_mostly")
>
>   #endif
>   /*
> diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h
> index 7968532..f4d9628 100644
> --- a/xen/include/asm-arm/percpu.h
> +++ b/xen/include/asm-arm/percpu.h
> @@ -19,7 +19,7 @@ void percpu_init_areas(void);
>
>   /* Separate out the type, so (int[3], foo) works. */
>   #define __DEFINE_PER_CPU(type, name, suffix)                    \
> -    __section(".bss.percpu" #suffix)                            \
> +    __section(".bss..percpu" #suffix)                            \
>       __typeof__(type) per_cpu_##name
>
>   #define per_cpu(var, cpu)  \
> diff --git a/xen/include/asm-x86/cache.h b/xen/include/asm-x86/cache.h
> index f4a08e7..5aedb85 100644
> --- a/xen/include/asm-x86/cache.h
> +++ b/xen/include/asm-x86/cache.h
> @@ -10,6 +10,6 @@
>   #define L1_CACHE_SHIFT	(CONFIG_X86_L1_CACHE_SHIFT)
>   #define L1_CACHE_BYTES	(1 << L1_CACHE_SHIFT)
>
> -#define __read_mostly __section(".data.read_mostly")
> +#define __read_mostly __section(".data..read_mostly")
>
>   #endif
> diff --git a/xen/include/asm-x86/percpu.h b/xen/include/asm-x86/percpu.h
> index 51562b9..5d442c3 100644
> --- a/xen/include/asm-x86/percpu.h
> +++ b/xen/include/asm-x86/percpu.h
> @@ -9,7 +9,7 @@ void percpu_init_areas(void);
>
>   /* Separate out the type, so (int[3], foo) works. */
>   #define __DEFINE_PER_CPU(type, name, suffix)                    \
> -    __section(".bss.percpu" #suffix)                            \
> +    __section(".bss..percpu" #suffix)                            \
>       __typeof__(type) per_cpu_##name
>
>   /* var is in discarded region: offset to particular copy we want */
> diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
> index bc0e794..78c6462 100644
> --- a/xen/include/xen/sched-if.h
> +++ b/xen/include/xen/sched-if.h
> @@ -175,7 +175,7 @@ struct scheduler {
>   };
>
>   #define REGISTER_SCHEDULER(x) static const struct scheduler *x##_entry \
> -  __used_section(".data.schedulers") = &x;
> +  __used_section(".data..schedulers") = &x;
>
>   struct cpupool
>   {
>

-- 
Julien Grall

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

Re: [PATCH v1 7/7] Rename sections for compatibility with -ffunction-sections -fdata-sections

[Jan Beulich]

>>> On 06.05.16 at 17:48, <ross.lagerwall@citrix.com> wrote:
> When building with -ffunction-sections -fdata-sections, it will generate
> section names like .text.show_handlers and .data.payload_list. These
> sections are in the same namespace as the special sections that Xen
> uses, such as .text.kexec and .data.schedulers. To prevent conflicts,
> prefix Xen's special sections with an extra period.
> 
> The idea for this was taken from a similar patch series applied to the
> Linux kernel by the kSplice folks.

First of all - I disliked this naming when I saw it in Linux, and hence
I dislike it here. These double dots just look odd, and they don't
really guard us against any future naming model gcc or clang may
come up with (nor does any other naming scheme, I agree, but I'd
still like a model not using dots better as being of lower risk collision
wise). Which gets me to the first question: What's really the
problem with names generate with -f{function,data}-sections
colliding with ours? I can't see much going wrong as long as all
objects have their alignment suitably recorded, with a few
exceptions, e.g. ...

> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -74,11 +74,11 @@ SECTIONS
>  
>    .data : {                    /* Data */
>         . = ALIGN(PAGE_SIZE);
> -       *(.data.page_aligned)
> +       *(.data..page_aligned)
>         *(.data)
>         . = ALIGN(8);
>         __start_schedulers_array = .;
> -       *(.data.schedulers)
> +       *(.data..schedulers)
>         __end_schedulers_array = .;

... things getting enclosed in labels.

>         *(.data.rel)

Why is .data.rel not an issue? A writable global variable named "rel"
would end up in a section named .data.rel with -fdata-sections. IOW
it may well be that a single .lds file serving both purposes can't be
used, unless you want to communicate the use of
-f{function,data}-sections here to make such directives conditional.

> @@ -173,15 +173,15 @@ SECTIONS
>  
>    .bss : {                     /* BSS */
>         __bss_start = .;
> -       *(.bss.stack_aligned)
> +       *(.bss..stack_aligned)
>         . = ALIGN(PAGE_SIZE);
> -       *(.bss.page_aligned)
> +       *(.bss..page_aligned)
>         *(.bss)
>         . = ALIGN(SMP_CACHE_BYTES);
>         __per_cpu_start = .;
> -       *(.bss.percpu)
> +       *(.bss..percpu)
>         . = ALIGN(SMP_CACHE_BYTES);
> -       *(.bss.percpu.read_mostly)
> +       *(.bss..percpu.read_mostly)

Nor do I think names which already have a 3rd dot in them are a
problem with the current naming scheme.

> --- a/xen/arch/x86/boot/x86_64.S
> +++ b/xen/arch/x86/boot/x86_64.S
> @@ -51,7 +51,7 @@ GLOBAL(gdt_descr)
>  GLOBAL(stack_start)
>          .quad   cpu0_stack
>  
> -        .section .data.page_aligned, "aw", @progbits
> +        .section .data..page_aligned, "aw", @progbits

And then, now that you want to go through this exercise and
rename all of them, I think this should be done by introducing a
suitable abstraction, such that another similar rename in the
future can be done by fiddling with a few #define-s in a central
place, without having to go though and touch random sources.
These #define-s could then also be paired with ones usable in
the *.lds files to pick up exactly those section names.

Jan


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

Re: [PATCH v1 1/7] lib: Add a generic implementation of current_text_addr()

[Jan Beulich]

>>> On 06.05.16 at 17:48, <ross.lagerwall@citrix.com> wrote:
> --- a/xen/common/lib.c
> +++ b/xen/common/lib.c
> @@ -499,6 +499,18 @@ void __init init_constructors(void)
>  }
>  
>  /*
> + * The GCC docs state that the function must be marked noinline to have the
> + * expected result:
> + * "When inlining the expected behavior is that the function returns the
> + * address of the function that is returned to. To work around this behavior
> + * use the noinline function attribute."
> + */
> +noinline void *current_text_addr(void)
> +{
> +    return __builtin_return_address(0);
> +}

If this is to become common code (which I'm fine with), I think you
should wrap it with __builtin_extract_return_addr() as per the
gcc documentation. And considering the return value points into
text (i.e. generally read-only memory) please make the return
type const void * unless there are strong reasons against doing
so.

Jan


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

Re: [PATCH v1 2/7] sched: Remove dependency on __LINE__ for release builds

[Jan Beulich]

>>> On 06.05.16 at 17:48, <ross.lagerwall@citrix.com> wrote:
> When using xsplice, use of __LINE__ can generate spurious changes in
> functions due to embedded line numbers.  For release builds, remove the
> use of these line numbers in domain_crash*() and print the current text
> address instead.

Which makes it more cumbersome to look up the origin. At the very
least ...

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -611,20 +611,34 @@ void vcpu_end_shutdown_deferral(struct vcpu *v);
>   * from any processor.
>   */
>  void __domain_crash(struct domain *d);
> +#ifdef NDEBUG

... should this imo become "defined(NDEBUG) &&
defined(CONFIG_XSPLICE)" then.

> +#define domain_crash(d) do {                                              \
> +    printk("domain_crash called from %p\n", current_text_addr());         \

And I'd really expect %ps or %pS to be used here.

> +    __domain_crash(d);                                                    \
> +} while (0)
> +#else
>  #define domain_crash(d) do {                                              \
>      printk("domain_crash called from %s:%d\n", __FILE__, __LINE__);       \
>      __domain_crash(d);                                                    \
>  } while (0)
> +#endif

Furthermore, considering this basically repeats ...

> +#ifdef NDEBUG
> +#define domain_crash_synchronous() do {                                   \
> +    printk("domain_crash_sync called from %p\n", current_text_addr());    \
> +    __domain_crash_synchronous();                                         \
> +} while (0)
> +#else
>  #define domain_crash_synchronous() do {                                   \
>      printk("domain_crash_sync called from %s:%d\n", __FILE__, __LINE__);  \
>      __domain_crash_synchronous();                                         \
>  } while (0)
> +#endif

... here, please limit the #ifdef-ery by abstracting just the
printk() invocation in a way that makes it possible to be used in
both places.

Jan


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

Re: [PATCH v1 4/7] page-alloc: Remove dependency on __LINE__ for release builds

[Andrew Cooper]

On 06/05/16 16:48, Ross Lagerwall wrote:
> When using xsplice, use of __LINE__ can generate spurious changes in
> functions due to embedded line numbers.  For release builds, remove the
> use of these line numbers in BOOT_BUG_ON() and print the current text
> address instead.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

I would just drop boot_bug() and BOOT_BUG_ON(), and replace them with
straight panic() calls.  It is only 3 uses in total.

The line numbers are really less helpful than a good description of the
reason for a panic.

~Andrew


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

Re: [PATCH v1 3/7] mm: Use statically defined locking order

[Jan Beulich]

>>> On 06.05.16 at 17:48, <ross.lagerwall@citrix.com> wrote:
> @@ -201,11 +203,20 @@ static inline void mm_enforce_order_unlock(int unlock_level,
>  
>  /************************************************************************
>   *                                                                      *
> - * To avoid deadlocks, these locks _MUST_ be taken in the order they're *
> - * declared in this file.  The locking functions will enforce this.     *
> + * To avoid deadlocks, these locks _MUST_ be taken in the order listed  *
> + * below.  The locking functions will enforce this.                     *
>   *                                                                      *
>   ************************************************************************/
>  
> +#define MM_LOCK_ORDER_nestedp2m              10000
> +#define MM_LOCK_ORDER_p2m                    20000
> +#define MM_LOCK_ORDER_altp2mlist             30000
> +#define MM_LOCK_ORDER_altp2m                 40000
> +#define MM_LOCK_ORDER_per_page_sharing       50000
> +#define MM_LOCK_ORDER_pod                    60000
> +#define MM_LOCK_ORDER_page_alloc             70000
> +#define MM_LOCK_ORDER_paging                 80000

It would seem more natural for these to appear ahead of being used,
even if the order of #define-s doesn't really matter. And then, why
multiples of 10000? Large numbers are generally less efficient to deal
with (i.e. they can't be fit in sign-extended 8-bit immediates).

Jan


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

Re: [PATCH v1 3/7] mm: Use statically defined locking order

[Andrew Cooper]

On 06/05/16 16:48, Ross Lagerwall wrote:
> Instead of using a locking order based on line numbers which doesn't
> play nicely with xSplice, statically define the locking order.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Along with this change, it would useful to modify the message in
__check_lock_level() to explicitly refer to mm-locks.h

The current message is very obscure to anyone who doesn't know about
mm-locks.h

~Andrew

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

Re: [PATCH v1 4/7] page-alloc: Remove dependency on __LINE__ for release builds

[Jan Beulich]

>>> On 06.05.16 at 17:48, <ross.lagerwall@citrix.com> wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -206,11 +206,19 @@ struct scrub_region {
>  static struct scrub_region __initdata region[MAX_NUMNODES];
>  static unsigned long __initdata chunk_size;
>  
> +#ifdef NDEBUG
> +static void __init boot_bug(void *addr)
> +{
> +    panic("Boot BUG at %p", addr);
> +}
> +#define BOOT_BUG_ON(p) if ( p ) boot_bug(current_text_addr());
> +#else
>  static void __init boot_bug(int line)
>  {
>      panic("Boot BUG at %s:%d", __FILE__, line);
>  }
>  #define BOOT_BUG_ON(p) if ( p ) boot_bug(__LINE__);
> +#endif

Along the lines of the comment on the earlier patch, please try
to limit the #ifdef to just the panic invocation. The function can
easily take both parameters and use just one in each variant.
And of course %ps or %pS and a dependency on CONFIG_XSPLICE
again.

Jan


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

Re: [PATCH v1 5/7] iommu: Remove dependency on __LINE__ for release builds

[Jan Beulich]

>>> On 06.05.16 at 17:48, <ross.lagerwall@citrix.com> wrote:
> --- a/xen/drivers/passthrough/vtd/dmar.h
> +++ b/xen/drivers/passthrough/vtd/dmar.h
> @@ -108,6 +108,15 @@ struct acpi_atsr_unit *acpi_find_matched_atsr_unit(const struct pci_dev *);
>  
>  #define DMAR_OPERATION_TIMEOUT MILLISECS(1000)
>  
> +#ifdef NDEBUG
> +#define IOMMU_WAIT_OP_PANIC \
> +    panic("%p: DMAR hardware is malfunctional", current_text_addr());
> +#else
> +#define IOMMU_WAIT_OP_PANIC \
> +    panic("%s:%d:%s: DMAR hardware is malfunctional",  \
> +          __FILE__, __LINE__, __func__);
> +#endif

The semicolons belong ...

> @@ -117,8 +126,7 @@ do {                                                \
>              break;                                  \
>          if ( NOW() > start_time + DMAR_OPERATION_TIMEOUT ) {    \
>              if ( !kexecing )                                    \
> -                panic("%s:%d:%s: DMAR hardware is malfunctional",\
> -                      __FILE__, __LINE__, __func__);            \
> +                IOMMU_WAIT_OP_PANIC                             \

 ... here. Also I think this would look better as a function style
macro.

Jan


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

Re: [PATCH v1 6/7] acpi: Remove dependency on __LINE__ for release builds

[Jan Beulich]

>>> On 06.05.16 at 17:48, <ross.lagerwall@citrix.com> wrote:
> --- a/xen/drivers/acpi/utilities/utmisc.c
> +++ b/xen/drivers/acpi/utilities/utmisc.c
> @@ -134,6 +134,51 @@ const char *__init acpi_ut_validate_exception(acpi_status status)
>  *
>  ******************************************************************************/
>  
> +#ifdef NDEBUG
> +void ACPI_INTERNAL_VAR_XFACE __init
> +acpi_ut_error(const char *module_name, void *addr, char *format, ...)
> +{
> +	va_list args;
> +
> +	acpi_os_printf("ACPI Error (%s-%p): ", module_name, addr);
> +
> +	va_start(args, format);
> +	acpi_os_vprintf(format, args);
> +	acpi_os_printf(" [%X]\n", ACPI_CA_VERSION);
> +	va_end(args);
> +}
> +
> +void ACPI_INTERNAL_VAR_XFACE __init
> +acpi_ut_warning(const char *module_name, void *addr, char *format, ...)
> +{
> +	va_list args;
> +
> +	acpi_os_printf("ACPI Warning (%s-%p): ", module_name, addr);
> +
> +	va_start(args, format);
> +	acpi_os_vprintf(format, args);
> +	acpi_os_printf(" [%X]\n", ACPI_CA_VERSION);
> +	va_end(args);
> +	va_end(args);
> +}
> +
> +void ACPI_INTERNAL_VAR_XFACE __init
> +acpi_ut_info(const char *module_name, void *addr, char *format, ...)
> +{
> +	va_list args;
> +
> +	/*
> +	 * Removed module_name, line_number, and acpica version, not needed
> +	 * for info output
> +	 */
> +	acpi_os_printf("ACPI: ");
> +
> +	va_start(args, format);
> +	acpi_os_vprintf(format, args);
> +	acpi_os_printf("\n");
> +	va_end(args);
> +}
> +#else
>  void ACPI_INTERNAL_VAR_XFACE __init
>  acpi_ut_error(const char *module_name, u32 line_number, char *format, ...)
>  {

This is all __init code, runtime patching of which makes no sense
anyway (as it's gone by the time a patch can get applied), so
this patch seems pointless. And if it was nevertheless needed for
a reason I overlook right now, it's way too much redundant code
that gets added, i.e. proper abstraction to limit the impact would
be necessary.

Jan

Jan


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

Re: [PATCH v1 0/7] Make building xSplice patches easier

[Konrad Rzeszutek Wilk]

On Fri, May 06, 2016 at 04:48:01PM +0100, Ross Lagerwall wrote:
> Here is a set of changes to make building xSplice patches easier.
> Tested to boot on x86.
> Compile-tested on arm.
> 
> This is probably too late to make it into 4.7, but hey, if someone wants
> to put it in I've CC'd Wei.

Hey,

I've put it on my #xsplice.v11.rfc branch, along with the two patches that didn't make
it in 4.7. And some of the prototype work on ARM..

git://xenbits.xen.org/people/konradwilk/xen.git xsplice.v11.rfc


> 
> Ross Lagerwall (7):
>   lib: Add a generic implementation of current_text_addr()
>   sched: Remove dependency on __LINE__ for release builds
>   mm: Use statically defined locking order
>   page-alloc: Remove dependency on __LINE__ for release builds
>   iommu: Remove dependency on __LINE__ for release builds
>   acpi: Remove dependency on __LINE__ for release builds
>   Rename sections for compatibility with -ffunction-sections
>     -fdata-sections
> 
>  xen/arch/arm/xen.lds.S              | 14 +++++------
>  xen/arch/x86/boot/x86_64.S          |  2 +-
>  xen/arch/x86/hvm/hvm.c              |  2 +-
>  xen/arch/x86/mm.c                   |  4 ++--
>  xen/arch/x86/mm/mm-locks.h          | 29 +++++++++++++++--------
>  xen/arch/x86/setup.c                |  2 +-
>  xen/arch/x86/x86_64/kexec_reloc.S   |  2 +-
>  xen/arch/x86/xen.lds.S              | 18 +++++++--------
>  xen/common/lib.c                    | 12 ++++++++++
>  xen/common/page_alloc.c             |  8 +++++++
>  xen/drivers/acpi/utilities/utmisc.c | 46 +++++++++++++++++++++++++++++++++++++
>  xen/drivers/passthrough/vtd/dmar.h  | 12 ++++++++--
>  xen/include/acpi/acmacros.h         |  4 ++++
>  xen/include/acpi/acutils.h          | 19 +++++++++++++++
>  xen/include/asm-arm/cache.h         |  2 +-
>  xen/include/asm-arm/percpu.h        |  2 +-
>  xen/include/asm-x86/cache.h         |  2 +-
>  xen/include/asm-x86/percpu.h        |  2 +-
>  xen/include/asm-x86/processor.h     | 10 --------
>  xen/include/xen/lib.h               |  2 ++
>  xen/include/xen/sched-if.h          |  2 +-
>  xen/include/xen/sched.h             | 14 +++++++++++
>  22 files changed, 162 insertions(+), 48 deletions(-)
> 
> -- 
> 2.4.3
> 

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

Re: [PATCH v1 0/7] Make building xSplice patches easier

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 423 bytes --]

On 5/6/16 10:48 AM, Ross Lagerwall wrote:
> Here is a set of changes to make building xSplice patches easier.
> Tested to boot on x86.
> Compile-tested on arm.
> 
> This is probably too late to make it into 4.7, but hey, if someone wants
> to put it in I've CC'd Wei.

Ross,

What happened with this series? Some of these patches still appear
un-applied and they appear relevant still.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

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

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

Re: [PATCH v1 0/7] Make building xSplice patches easier

[Ross Lagerwall]

On 01/31/2017 03:05 AM, Doug Goldstein wrote:
> On 5/6/16 10:48 AM, Ross Lagerwall wrote:
>> Here is a set of changes to make building xSplice patches easier.
>> Tested to boot on x86.
>> Compile-tested on arm.
>>
>> This is probably too late to make it into 4.7, but hey, if someone wants
>> to put it in I've CC'd Wei.
>
> Ross,
>
> What happened with this series? Some of these patches still appear
> un-applied and they appear relevant still.
>

Sorry, I got side-tracked with other things. I will refresh and send a 
new version soon.

Cheers,
-- 
Ross Lagerwall

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