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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/10/17 16:53, Jan Beulich wrote:

On 02.10.17 at 18:13, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxx>

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

~Andrew

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