Re: [PATCH for-next 2/3] xen/x86: Introduce static inline wrappers for l{idt, gdt, ldt, tr}()

[Andrew Cooper <andrew.cooper3@citrix.com>]

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

On 12/10/17 16:53, Jan Beulich wrote:
>>>> On 02.10.17 at 18:13, <andrew.cooper3@citrix.com> wrote:
>> The triple-fault reboot method stays as is, to avoid the int3 possibly getting
>> moved relative to the lidt.
> Aren't asm volatile()s ordered wrt to one another?

>From the docs

"Note that the compiler can move even volatile |asm| instructions
relative to other code, including across jump instructions."

Also, I seem to recall Tim finding an example where GCC 6 did reorder
two asm volatiles relative to each other, due to their output operands
not being causally linked.

On that note however, these should gain memory clobbers to make them
full barriers.  l{i,g}dt() are serialising, while nothing good will come
of having a segment register access reordered with respect to l{g,l}dt().

>
>> --- a/xen/include/asm-x86/desc.h
>> +++ b/xen/include/asm-x86/desc.h
>> @@ -197,6 +197,26 @@ DECLARE_PER_CPU(struct desc_struct *, compat_gdt_table);
>>  
>>  extern void load_TR(void);
>>  
>> +static inline void lgdt(const struct desc_ptr *gdtr)
>> +{
>> +    asm volatile ("lgdt %0" :: "m" (*gdtr));
>> +}
>> +
>> +static inline void lidt(const struct desc_ptr *idtr)
>> +{
>> +    asm volatile ("lidt %0" :: "m" (*idtr));
>> +}
>> +
>> +static inline void lldt(unsigned int sel)
>> +{
>> +    asm volatile ("lldt %w0" :: "rm" (sel));
>> +}
>> +
>> +static inline void ltr(unsigned int sel)
>> +{
>> +    asm volatile ("ltr %w0" :: "rm" (sel));
>> +}
> As can be seen from the code you replace in ldt.h, in headers we
> generally prefer to use __asm__ (and __volatile__ where needed).
> I'm sure this isn't consistent, so I won't insist. However, style-wise
> please add blanks immediately inside the parentheses. With at least
> this last point taken care of

Will do.

> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Does this still stand in light of the barrier change above?

~Andrew

[-- Attachment #1.2: Type: text/html, Size: 3071 bytes --]

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

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