[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.