Re: [Xen-devel] [PATCH] mm: fix LLVM code-generation issue

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.11.18 at 13:38, <julien.grall@xxxxxxx> wrote:
> Hi Andrew,
> 
> On 11/22/18 12:23 PM, Andrew Cooper wrote:
>> On 22/11/2018 12:03, Roger Pau Monne wrote:
>>> LLVM code generation can attempt to perform a 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>
>> 
>> You're actually missing two pieces here.  If I recall the disassembly
>> correctly, there was a `sete %al`, and an `and %al, -0x29(%rbp)` which
>> had the cumulative effect of calculating `idle_scrub` branchlessly
>> (which is no doubt the intended optimisation).
>> 
>>>
>>> Such code will trigger a page fault if system_state >=
>>> SYS_STATE_active.
>>>
>>> In order to prevent such optimization signal to the compiler that
>>> accessing opt_bootscrub can have side effects by using ACCESS_ONCE.
>>> This has been reported and discussed with upstream LLVM:
>>>
>>> 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@xxxxxxxxxx>
>> 
>> To unblock the Clang build, Acked-by: Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>, although ideally with the expanded disassembly.
> 
> Actually is it enough in all the case? We are only preventing the 
> re-ordering for the compiler. The processor is still free to re-order it 
> and load opt_bootscrub before loading system_state.

The processor is fine to issue the load early, but it is not permitted
to raise an exception resulting from that read attempt before the
reading insn is the next one to retire.

Jan


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