[PATCH v2] mm: make opt_bootscrub non-init

* [PATCH v2] mm: make opt_bootscrub non-init
@ 2018-11-26 17:55 Roger Pau Monne
  2018-11-27 10:13 ` Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Roger Pau Monne @ 2018-11-26 17:55 UTC (permalink / raw)
  To: xen-devel
  Cc: Sergey Dyasli, Stefano Stabellini, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	Tim Deegan, Julien Grall, Jan Beulich, Roger Pau Monne

LLVM code generation can attempt to load from a variable in the next
condition of an expression under certain circumstances, thus turning
the following condition:

if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE )

Into:

0xffff82d080223967 <+103>: cmpl   $0x3,0x37b032(%rip) # 0xffff82d08059e9a0 <system_state>
0xffff82d08022396e <+110>: setb   -0x29(%rbp)
0xffff82d080223972 <+114>: cmpl   $0x2,0x228a8b(%rip) # 0xffff82d08044c404 <opt_bootscrub>

Such code will trigger a page fault if system_state >=
SYS_STATE_active because opt_bootscrub will be unmapped.

Fix this by making opt_bootscrub non-init, thus preventing the page
fault. The LLVM bug with the discussion about this issue can be found
at:

https://bugs.llvm.org/show_bug.cgi?id=39707

I haven't been able to find any other instances of such conditional
expression that uses system_state together with an init variable or
function.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Sergey Dyasli <sergey.dyasli@citrix.com>
---
Changes since v1:
 - Make opt_bootscrub read mostly.
 - Add a comment about why opt_bootscrub is not in the init section.
---
 xen/common/page_alloc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 08ee8cfbb9..b4086781c4 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -166,7 +166,14 @@ enum bootscrub_mode {
     BOOTSCRUB_ON,
     BOOTSCRUB_IDLE,
 };
-static enum bootscrub_mode __initdata opt_bootscrub = BOOTSCRUB_IDLE;
+/*
+ * opt_bootscrub should live in the init section, since it's not accessed
+ * afterwards. However at least LLVM assumes there are no side effects of
+ * accessing the variable, and optimizes the condition so opt_bootscrub is
+ * read regardless of the value of system_state:
+ * https://bugs.llvm.org/show_bug.cgi?id=39707
+ */
+static enum bootscrub_mode __read_mostly opt_bootscrub = BOOTSCRUB_IDLE;
 static int __init parse_bootscrub_param(const char *s)
 {
     /* Interpret 'bootscrub' alone in its positive boolean form */
-- 
2.19.1


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

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