* [PATCH] KVM: Avoid using vmx instruction directly
@ 2006-11-09 11:08 Avi Kivity
2006-11-09 13:29 ` Arnd Bergmann
0 siblings, 1 reply; 15+ messages in thread
From: Avi Kivity @ 2006-11-09 11:08 UTC (permalink / raw)
To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
Cc: akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Some users have an older assembler installed which doesn't grok the
vmx instructions.
Fix by encoding the instruction opcodes directly.
Signed-off-by: Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
Index: linux-2.6/drivers/kvm/kvm.h
===================================================================
--- linux-2.6.orig/drivers/kvm/kvm.h
+++ linux-2.6/drivers/kvm/kvm.h
@@ -377,6 +377,16 @@ static inline struct kvm_mmu_page *page_
return (struct kvm_mmu_page *)page->private;
}
+#define ASM_VMX_VMCLEAR_RAX ".byte 0x66, 0x0f, 0xc7, 0x30"
+#define ASM_VMX_VMLAUNCH ".byte 0x0f, 0x01, 0xc2"
+#define ASM_VMX_VMRESUME ".byte 0x0f, 0x01, 0xc3"
+#define ASM_VMX_VMPTRLD_RAX ".byte 0x0f, 0xc7, 0x30"
+#define ASM_VMX_VMREAD_RDX_RAX ".byte 0x0f, 0x78, 0xd0"
+#define ASM_VMX_VMWRITE_RAX_RDX ".byte 0x0f, 0x79, 0xd0"
+#define ASM_VMX_VMWRITE_RSP_RDX ".byte 0x0f, 0x79, 0xd4"
+#define ASM_VMX_VMXOFF ".byte 0x0f, 0x01, 0xc4"
+#define ASM_VMX_VMXON_RAX ".byte 0xf3, 0x0f, 0xc7, 0x30"
+
#ifdef __x86_64__
/*
Index: linux-2.6/drivers/kvm/kvm_main.c
===================================================================
--- linux-2.6.orig/drivers/kvm/kvm_main.c
+++ linux-2.6/drivers/kvm/kvm_main.c
@@ -369,8 +369,8 @@ static void vmcs_clear(struct vmcs *vmcs
u64 phys_addr = __pa(vmcs);
u8 error;
- asm volatile ("vmclear %1; setna %0"
- : "=m"(error) : "m"(phys_addr) : "cc", "memory" );
+ asm volatile (ASM_VMX_VMCLEAR_RAX "; setna %0"
+ : "=g"(error) : "a"(&phys_addr) : "cc", "memory" );
if (error)
printk(KERN_ERR "kvm: vmclear fail: %p/%llx\n",
vmcs, phys_addr);
@@ -412,8 +412,8 @@ static struct kvm_vcpu *__vcpu_load(stru
u8 error;
per_cpu(current_vmcs, cpu) = vcpu->vmcs;
- asm volatile ("vmptrld %1; setna %0"
- : "=m"(error) : "m"(phys_addr) : "cc" );
+ asm volatile (ASM_VMX_VMPTRLD_RAX "; setna %0"
+ : "=g"(error) : "a"(&phys_addr) : "cc" );
if (error)
printk(KERN_ERR "kvm: vmptrld %p/%llx fail\n",
vcpu->vmcs, phys_addr);
@@ -536,12 +536,12 @@ static __init void kvm_enable(void *garb
/* enable and lock */
wrmsrl(MSR_IA32_FEATURE_CONTROL, old | 5);
write_cr4(read_cr4() | CR4_VMXE); /* FIXME: not cpu hotplug safe */
- asm volatile ("vmxon %0" : : "m"(phys_addr) : "memory", "cc");
+ asm volatile (ASM_VMX_VMXON_RAX : : "a"(&phys_addr) : "memory", "cc");
}
static void kvm_disable(void *garbage)
{
- asm volatile ("vmxoff" : : : "cc");
+ asm volatile (ASM_VMX_VMXOFF : : : "cc");
}
static int kvm_dev_open(struct inode *inode, struct file *filp)
@@ -633,7 +633,8 @@ unsigned long vmcs_readl(unsigned long f
{
unsigned long value;
- asm volatile ("vmread %1, %0" : "=g"(value) : "r"(field) : "cc");
+ asm volatile (ASM_VMX_VMREAD_RDX_RAX
+ : "=a"(value) : "d"(field) : "cc");
return value;
}
@@ -641,8 +642,8 @@ void vmcs_writel(unsigned long field, un
{
u8 error;
- asm volatile ("vmwrite %1, %2; setna %0"
- : "=g"(error) : "r"(value), "r"(field) : "cc" );
+ asm volatile (ASM_VMX_VMWRITE_RAX_RDX "; setna %0"
+ : "=q"(error) : "a"(value), "d"(field) : "cc" );
if (error)
printk(KERN_ERR "vmwrite error: reg %lx value %lx (err %d)\n",
field, value, vmcs_read32(VM_INSTRUCTION_ERROR));
@@ -2634,10 +2635,10 @@ again:
"push %%r8; push %%r9; push %%r10; push %%r11;"
"push %%r12; push %%r13; push %%r14; push %%r15;"
"push %%rcx \n\t"
- "vmwrite %%rsp, %2 \n\t"
+ ASM_VMX_VMWRITE_RSP_RDX "\n\t"
#else
"pusha; push %%ecx \n\t"
- "vmwrite %%esp, %2 \n\t"
+ ASM_VMX_VMWRITE_RSP_RDX "\n\t"
#endif
/* Check if vmlaunch of vmresume is needed */
"cmp $0, %1 \n\t"
@@ -2673,9 +2674,9 @@ again:
#endif
/* Enter guest mode */
"jne launched \n\t"
- "vmlaunch \n\t"
+ ASM_VMX_VMLAUNCH "\n\t"
"jmp kvm_vmx_return \n\t"
- "launched: vmresume \n\t"
+ "launched: " ASM_VMX_VMRESUME "\n\t"
".globl kvm_vmx_return \n\t"
"kvm_vmx_return: "
/* Save guest registers, load host registers, keep flags */
@@ -2722,7 +2723,7 @@ again:
"setbe %0 \n\t"
"popf \n\t"
: "=g" (fail)
- : "r"(vcpu->launched), "r"((unsigned long)HOST_RSP),
+ : "r"(vcpu->launched), "d"((unsigned long)HOST_RSP),
"c"(vcpu),
[rax]"i"(offsetof(struct kvm_vcpu, regs[VCPU_REGS_RAX])),
[rbx]"i"(offsetof(struct kvm_vcpu, regs[VCPU_REGS_RBX])),
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] KVM: Avoid using vmx instruction directly 2006-11-09 11:08 [PATCH] KVM: Avoid using vmx instruction directly Avi Kivity @ 2006-11-09 13:29 ` Arnd Bergmann [not found] ` <200611091429.42040.arnd-r2nGTMty4D4@public.gmane.org> 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2006-11-09 13:29 UTC (permalink / raw) To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Cc: akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thursday 09 November 2006 12:08, Avi Kivity wrote: > Index: linux-2.6/drivers/kvm/kvm_main.c > =================================================================== > --- linux-2.6.orig/drivers/kvm/kvm_main.c > +++ linux-2.6/drivers/kvm/kvm_main.c > @@ -369,8 +369,8 @@ static void vmcs_clear(struct vmcs *vmcs > u64 phys_addr = __pa(vmcs); > u8 error; > > - asm volatile ("vmclear %1; setna %0" > - : "=m"(error) : "m"(phys_addr) : "cc", "memory" ); > + asm volatile (ASM_VMX_VMCLEAR_RAX "; setna %0" > + : "=g"(error) : "a"(&phys_addr) : "cc", "memory" ); > if (error) > printk(KERN_ERR "kvm: vmclear fail: %p/%llx\n", > vmcs, phys_addr); I'm not an expert on inline assembly, but don't you need an extra '"m" (phys_addr)' to make sure that gcc actually puts the variable on the stack instead of passing a NULL pointer as '"a"(&phys_addr)'? Arnd <>< ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <200611091429.42040.arnd-r2nGTMty4D4@public.gmane.org>]
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <200611091429.42040.arnd-r2nGTMty4D4@public.gmane.org> @ 2006-11-09 13:36 ` Avi Kivity [not found] ` <45532EE3.4000104-atKUWr5tajBWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 15+ messages in thread From: Avi Kivity @ 2006-11-09 13:36 UTC (permalink / raw) To: Arnd Bergmann Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA Arnd Bergmann wrote: > On Thursday 09 November 2006 12:08, Avi Kivity wrote: > >> Index: linux-2.6/drivers/kvm/kvm_main.c >> =================================================================== >> --- linux-2.6.orig/drivers/kvm/kvm_main.c >> +++ linux-2.6/drivers/kvm/kvm_main.c >> @@ -369,8 +369,8 @@ static void vmcs_clear(struct vmcs *vmcs >> u64 phys_addr = __pa(vmcs); >> u8 error; >> >> - asm volatile ("vmclear %1; setna %0" >> - : "=m"(error) : "m"(phys_addr) : "cc", "memory" ); >> + asm volatile (ASM_VMX_VMCLEAR_RAX "; setna %0" >> + : "=g"(error) : "a"(&phys_addr) : "cc", "memory" ); >> if (error) >> printk(KERN_ERR "kvm: vmclear fail: %p/%llx\n", >> vmcs, phys_addr); >> > > I'm not an expert on inline assembly, but don't you need an extra > '"m" (phys_addr)' to make sure that gcc actually puts the variable > on the stack instead of passing a NULL pointer as '"a"(&phys_addr)'? > > Taking a variable's address should force its contents into memory (like calling an uninlined function with &var). -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <45532EE3.4000104-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <45532EE3.4000104-atKUWr5tajBWk0Htik3J/w@public.gmane.org> @ 2006-11-09 14:42 ` Arnd Bergmann 2006-11-09 14:52 ` [kvm-devel] " Avi Kivity 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2006-11-09 14:42 UTC (permalink / raw) To: Avi Kivity Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thursday 09 November 2006 14:36, Avi Kivity wrote: > > > > > I'm not an expert on inline assembly, but don't you need an extra > > '"m" (phys_addr)' to make sure that gcc actually puts the variable > > on the stack instead of passing a NULL pointer as '"a"(&phys_addr)'? > > Taking a variable's address should force its contents into memory (like > calling an uninlined function with &var). No it doesn't. You're not telling gcc that the inline assembly cares about the contents of the variable, so it could be a reference to a stack slot while the contents are still in a register. Or gcc might move the assignment of phys_addr to after the inline assembly. Arnd <>< ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [kvm-devel] [PATCH] KVM: Avoid using vmx instruction directly 2006-11-09 14:42 ` Arnd Bergmann @ 2006-11-09 14:52 ` Avi Kivity 2006-11-09 16:37 ` Arnd Bergmann 2006-11-09 23:39 ` Jeremy Fitzhardinge 0 siblings, 2 replies; 15+ messages in thread From: Avi Kivity @ 2006-11-09 14:52 UTC (permalink / raw) To: Arnd Bergmann; +Cc: kvm-devel, akpm, linux-kernel Arnd Bergmann wrote: > On Thursday 09 November 2006 14:36, Avi Kivity wrote: > >>> I'm not an expert on inline assembly, but don't you need an extra >>> '"m" (phys_addr)' to make sure that gcc actually puts the variable >>> on the stack instead of passing a NULL pointer as '"a"(&phys_addr)'? >>> >> Taking a variable's address should force its contents into memory (like >> calling an uninlined function with &var). >> > > No it doesn't. You're not telling gcc that the inline assembly cares > about the contents of the variable, so it could be a reference to > a stack slot while the contents are still in a register. Wouldn't that make inline assembly useless? Suppose the contents is itself a pointer. What about the pointed-to contents? e.g. int x = 3; int *y = &x; int z; asm ("mov %1, %%rax; movl (%%rax), %0" : "=r"(z) : "g"(y) : "rax"); assert(z == 3); > Or gcc > might move the assignment of phys_addr to after the inline assembly. > "asm volatile" prevents that (and I'm not 100% sure it's necessary). -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [kvm-devel] [PATCH] KVM: Avoid using vmx instruction directly 2006-11-09 14:52 ` [kvm-devel] " Avi Kivity @ 2006-11-09 16:37 ` Arnd Bergmann 2006-11-09 16:51 ` Avi Kivity 2006-11-09 23:39 ` Jeremy Fitzhardinge 1 sibling, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2006-11-09 16:37 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm-devel, akpm, linux-kernel On Thursday 09 November 2006 15:52, Avi Kivity wrote: > Wouldn't that make inline assembly useless? Suppose the contents is > itself a pointer. What about the pointed-to contents? > > e.g. > > int x = 3; > int *y = &x; > int z; > > asm ("mov %1, %%rax; movl (%%rax), %0" : "=r"(z) : "g"(y) : "rax"); > assert(z == 3); Same here, you need to tell gcc what is really accessed, like asm ("mov %1, %%rax; movl (%%rax), %0" : "=r"(z) : "g"(y), "m"(*y) : "rax"); I know that the s390 kernel developers have hit that problem frequently with inline assemblies. It may be that it's harder to hit on x86, because there are fewer registers available and data therefore tends to spill to the stack. > > Or gcc > > might move the assignment of phys_addr to after the inline assembly. > > > "asm volatile" prevents that (and I'm not 100% sure it's necessary). Yes, I think that's right. The 'volatile' should not be necessary though, if you get the inputs right. Arnd <>< ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [kvm-devel] [PATCH] KVM: Avoid using vmx instruction directly 2006-11-09 16:37 ` Arnd Bergmann @ 2006-11-09 16:51 ` Avi Kivity 0 siblings, 0 replies; 15+ messages in thread From: Avi Kivity @ 2006-11-09 16:51 UTC (permalink / raw) To: Arnd Bergmann; +Cc: kvm-devel, akpm, linux-kernel Arnd Bergmann wrote: > On Thursday 09 November 2006 15:52, Avi Kivity wrote: > >> Wouldn't that make inline assembly useless? Suppose the contents is >> itself a pointer. What about the pointed-to contents? >> >> e.g. >> >> int x = 3; >> int *y = &x; >> int z; >> >> asm ("mov %1, %%rax; movl (%%rax), %0" : "=r"(z) : "g"(y) : "rax"); >> assert(z == 3); >> > > Same here, you need to tell gcc what is really accessed, like > > asm ("mov %1, %%rax; movl (%%rax), %0" : "=r"(z) : "g"(y), "m"(*y) : "rax"); > > I know that the s390 kernel developers have hit that problem > frequently with inline assemblies. It may be that it's harder > to hit on x86, because there are fewer registers available and > data therefore tends to spill to the stack. > I'll update my tree to reflect this. Thanks. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [kvm-devel] [PATCH] KVM: Avoid using vmx instruction directly 2006-11-09 14:52 ` [kvm-devel] " Avi Kivity 2006-11-09 16:37 ` Arnd Bergmann @ 2006-11-09 23:39 ` Jeremy Fitzhardinge 2006-11-10 12:46 ` Martin Schwidefsky [not found] ` <4553BC18.6090207-TSDbQ3PG+2Y@public.gmane.org> 1 sibling, 2 replies; 15+ messages in thread From: Jeremy Fitzhardinge @ 2006-11-09 23:39 UTC (permalink / raw) To: Avi Kivity; +Cc: Arnd Bergmann, kvm-devel, akpm, linux-kernel Avi Kivity wrote: >> Or gcc >> might move the assignment of phys_addr to after the inline assembly. >> > "asm volatile" prevents that (and I'm not 100% sure it's necessary). No, it won't necessarily. "asm volatile" simply forces gcc to emit the assembler, even if it thinks its output doesn't get used. It makes no ordering guarantees with respect to other code (or even other "asm volatiles"). The "memory" clobbers should fix the ordering of the asms though. J ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [kvm-devel] [PATCH] KVM: Avoid using vmx instruction directly 2006-11-09 23:39 ` Jeremy Fitzhardinge @ 2006-11-10 12:46 ` Martin Schwidefsky [not found] ` <6e0cfd1d0611100446j77a27b29jc23f76a515451377-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> [not found] ` <4553BC18.6090207-TSDbQ3PG+2Y@public.gmane.org> 1 sibling, 1 reply; 15+ messages in thread From: Martin Schwidefsky @ 2006-11-10 12:46 UTC (permalink / raw) To: Jeremy Fitzhardinge Cc: Avi Kivity, Arnd Bergmann, kvm-devel, akpm, linux-kernel On 11/10/06, Jeremy Fitzhardinge <jeremy@goop.org> wrote: > >> Or gcc > >> might move the assignment of phys_addr to after the inline assembly. > >> > > "asm volatile" prevents that (and I'm not 100% sure it's necessary). > > No, it won't necessarily. "asm volatile" simply forces gcc to emit the > assembler, even if it thinks its output doesn't get used. It makes no > ordering guarantees with respect to other code (or even other "asm > volatiles"). The "memory" clobbers should fix the ordering of the asms > though. The "memory" clobber just tells the compiler that any memory object might get access by the inline. This forces the compiler to write back values it cached in registers and to reload the values after the inline assembly. This does NOT make it generate correct code for local objects. We had the case where we created a control block on the stack and passed it to a magic instruction. Since we did not tell the compiler that the content of the control block is used but only the address of it, gcc just passed a local stack address to the inline but optimized the initialization of the control block away. So the following can break: struct control_block { int a, b; }; void fn(void) { struct control_block x; x.a = 42; x.b = 0815; asm volatile ("<magic>" : : "a" (&x) : "memory"); } You won't find the assignments to x.a and x.b in the compiled code. -- blue skies, Martin ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <6e0cfd1d0611100446j77a27b29jc23f76a515451377-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <6e0cfd1d0611100446j77a27b29jc23f76a515451377-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2006-11-10 19:38 ` Jeremy Fitzhardinge 0 siblings, 0 replies; 15+ messages in thread From: Jeremy Fitzhardinge @ 2006-11-10 19:38 UTC (permalink / raw) To: Martin Schwidefsky Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA Martin Schwidefsky wrote: > On 11/10/06, Jeremy Fitzhardinge <jeremy-TSDbQ3PG+2Y@public.gmane.org> wrote: >> >> Or gcc >> >> might move the assignment of phys_addr to after the inline assembly. >> >> >> > "asm volatile" prevents that (and I'm not 100% sure it's necessary). >> >> No, it won't necessarily. "asm volatile" simply forces gcc to emit the >> assembler, even if it thinks its output doesn't get used. It makes no >> ordering guarantees with respect to other code (or even other "asm >> volatiles"). The "memory" clobbers should fix the ordering of the asms >> though. > > The "memory" clobber just tells the compiler that any memory object > might get access by the inline. I just meant that two asms with a "memory" clobber will be generated with a fixed ordering, which "asm volatile" does not necessarily do. J ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <4553BC18.6090207-TSDbQ3PG+2Y@public.gmane.org>]
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <4553BC18.6090207-TSDbQ3PG+2Y@public.gmane.org> @ 2006-11-21 18:35 ` H. Peter Anvin [not found] ` <45634704.8020407-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 15+ messages in thread From: H. Peter Anvin @ 2006-11-21 18:35 UTC (permalink / raw) To: Jeremy Fitzhardinge Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA Jeremy Fitzhardinge wrote: > Avi Kivity wrote: >>> Or gcc >>> might move the assignment of phys_addr to after the inline assembly. >>> >> "asm volatile" prevents that (and I'm not 100% sure it's necessary). > > No, it won't necessarily. "asm volatile" simply forces gcc to emit the > assembler, even if it thinks its output doesn't get used. It makes no > ordering guarantees with respect to other code (or even other "asm > volatiles"). The "memory" clobbers should fix the ordering of the asms > though. > I think you're wrong about that; in particular, I'm pretty sure "asm volatiles" are ordered among themselves. What the "volatile" means is "this has side effects you (the compiler) don't understand", and gcc can't assume that it can reorder such side effects. -hpa ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <45634704.8020407-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <45634704.8020407-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> @ 2006-11-21 19:41 ` Avi Kivity 2006-11-21 20:50 ` Jeremy Fitzhardinge 1 sibling, 0 replies; 15+ messages in thread From: Avi Kivity @ 2006-11-21 19:41 UTC (permalink / raw) To: H. Peter Anvin Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, Jeremy Fitzhardinge, linux-kernel-u79uwXL29TY76Z2rM5mHXA H. Peter Anvin wrote: > Jeremy Fitzhardinge wrote: >> Avi Kivity wrote: >>>> Or gcc >>>> might move the assignment of phys_addr to after the inline assembly. >>>> >>> "asm volatile" prevents that (and I'm not 100% sure it's necessary). >> >> No, it won't necessarily. "asm volatile" simply forces gcc to emit the >> assembler, even if it thinks its output doesn't get used. It makes no >> ordering guarantees with respect to other code (or even other "asm >> volatiles"). The "memory" clobbers should fix the ordering of the asms >> though. >> > > I think you're wrong about that; in particular, I'm pretty sure "asm > volatiles" are ordered among themselves. What the "volatile" means is > "this has side effects you (the compiler) don't understand", and gcc > can't assume that it can reorder such side effects. The gcc manual has this to say: Similarly, you can't expect a sequence of volatile `asm' instructions to remain perfectly consecutive. If you want consecutive output, use a single `asm'. Also, GCC will perform some optimizations across a volatile `asm' instruction; GCC does not "forget everything" when it encounters a volatile `asm' instruction the way some other compilers do. I wonder how we are supposed to code the following sequence: asm volatile ("blah") /* sets funky processor mode */ some_c_code(); asm volatile ("unblah"); Let's say "blah" disables floating point exceptions, and some_c_code() must run without exceptions. Is is possible to code this in gcc without putting functions in another translation unit? Is a memory clobber sufficient? I'd certainly hate to use it. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <45634704.8020407-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org> 2006-11-21 19:41 ` Avi Kivity @ 2006-11-21 20:50 ` Jeremy Fitzhardinge [not found] ` <4563667B.2060209-TSDbQ3PG+2Y@public.gmane.org> 1 sibling, 1 reply; 15+ messages in thread From: Jeremy Fitzhardinge @ 2006-11-21 20:50 UTC (permalink / raw) To: H. Peter Anvin Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA H. Peter Anvin wrote: > I think you're wrong about that; in particular, I'm pretty sure "asm > volatiles" are ordered among themselves. What the "volatile" means is > "this has side effects you (the compiler) don't understand", and gcc > can't assume that it can reorder such side effects. That's not how I read the manual (quoted below). "asm volatile" is much weaker than people seem to think it is; the "volatile" puts fewer constraints on the compiler than it would for a variable. While the manual doesn't say that "asm volatiles" could be reordered with respect to each other, it doesn't say that they won't, and I don't see anything in this description which could be read to imply it (indeed "can be moved relative to other code" includes other asm volatiles). Like "volatile" variables, I think "asm volatile" is probably overused. If you want to guarantee specific ordering of asms, it's probably better to add an explicit dependency between them rather than rely on asm volatile; this could either be a "memory" clobber, or something more fine-grained. For example: /* need never be instansiated; never actually referenced */ extern int spin_sequencer; /* %0 never referenced */ asm("take spinlock" : "+m" (spin_sequencer)...); ... /* again, %0 never referenced */ asm("release spinlock" : "+m" (spin_sequencer)...); This is example is a bit contrived since a real spinlock would also have to have a memory clobber - or some other barrier - but you get the idea. It has the nice property of allowing you to define precise dependencies between various asm()s, without having to set up unnecessary dependencies between unrelated asms; and by making use of in/out/inout asm parameters, you can express different kinds of dependencies which give gcc a better chance of understanding what's going on. The relevent bit of the manual: The `volatile' keyword indicates that the instruction has important side-effects. GCC will not delete a volatile `asm' if it is reachable. (The instruction can still be deleted if GCC can prove that control-flow will never reach the location of the instruction.) Note that even a volatile `asm' instruction can be moved relative to other code, including across jump instructions. For example, on many targets there is a system register which can be set to control the rounding mode of floating point operations. You might try setting it with a volatile `asm', like this PowerPC example: asm volatile("mtfsf 255,%0" : : "f" (fpenv)); sum = x + y; This will not work reliably, as the compiler may move the addition back before the volatile `asm'. To make it work you need to add an artificial dependency to the `asm' referencing a variable in the code you don't want moved, for example: asm volatile ("mtfsf 255,%1" : "=X"(sum): "f"(fpenv)); sum = x + y; Similarly, you can't expect a sequence of volatile `asm' instructions to remain perfectly consecutive. If you want consecutive output, use a single `asm'. Also, GCC will perform some optimizations across a volatile `asm' instruction; GCC does not "forget everything" when it encounters a volatile `asm' instruction the way some other compilers do. An `asm' instruction without any output operands will be treated identically to a volatile `asm' instruction. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <4563667B.2060209-TSDbQ3PG+2Y@public.gmane.org>]
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <4563667B.2060209-TSDbQ3PG+2Y@public.gmane.org> @ 2006-11-22 6:42 ` Avi Kivity [not found] ` <4563F158.3060209-atKUWr5tajBWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 15+ messages in thread From: Avi Kivity @ 2006-11-22 6:42 UTC (permalink / raw) To: Jeremy Fitzhardinge Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA, H. Peter Anvin Jeremy Fitzhardinge wrote: > > > Like "volatile" variables, I think "asm volatile" is probably overused. > If you want to guarantee specific ordering of asms, it's probably better > to add an explicit dependency between them rather than rely on asm > volatile; this could either be a "memory" clobber, or something more > fine-grained. For example: > > /* need never be instansiated; never actually referenced */ > extern int spin_sequencer; > > /* %0 never referenced */ > asm("take spinlock" : "+m" (spin_sequencer)...); > > ... > > /* again, %0 never referenced */ > asm("release spinlock" : "+m" (spin_sequencer)...); > Very interesting. Will it work on load/store architectures? Since all memory access is through a register, won't the constraint generate a useless register load (and a use of the variable)? -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <4563F158.3060209-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH] KVM: Avoid using vmx instruction directly [not found] ` <4563F158.3060209-atKUWr5tajBWk0Htik3J/w@public.gmane.org> @ 2006-11-22 9:10 ` Jeremy Fitzhardinge 0 siblings, 0 replies; 15+ messages in thread From: Jeremy Fitzhardinge @ 2006-11-22 9:10 UTC (permalink / raw) To: Avi Kivity Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, akpm-3NddpPZAyC0, linux-kernel-u79uwXL29TY76Z2rM5mHXA, H. Peter Anvin Avi Kivity wrote: > Very interesting. > > Will it work on load/store architectures? Since all memory access is > through a register, won't the constraint generate a useless register > load (and a use of the variable)? Don't know; interesting question. It might be worth lobbying the gcc folks for an asm() constraint which means "pretend this is being read/written, but don't generate any code, and raise an error if the asm actually tries to use it". Or perhaps there's some way to do that already. On the other hand, load/store archs tend to have lots of registers anyway, so maybe it isn't a big deal. J ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2006-11-22 9:10 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-09 11:08 [PATCH] KVM: Avoid using vmx instruction directly Avi Kivity
2006-11-09 13:29 ` Arnd Bergmann
[not found] ` <200611091429.42040.arnd-r2nGTMty4D4@public.gmane.org>
2006-11-09 13:36 ` Avi Kivity
[not found] ` <45532EE3.4000104-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-11-09 14:42 ` Arnd Bergmann
2006-11-09 14:52 ` [kvm-devel] " Avi Kivity
2006-11-09 16:37 ` Arnd Bergmann
2006-11-09 16:51 ` Avi Kivity
2006-11-09 23:39 ` Jeremy Fitzhardinge
2006-11-10 12:46 ` Martin Schwidefsky
[not found] ` <6e0cfd1d0611100446j77a27b29jc23f76a515451377-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2006-11-10 19:38 ` Jeremy Fitzhardinge
[not found] ` <4553BC18.6090207-TSDbQ3PG+2Y@public.gmane.org>
2006-11-21 18:35 ` H. Peter Anvin
[not found] ` <45634704.8020407-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
2006-11-21 19:41 ` Avi Kivity
2006-11-21 20:50 ` Jeremy Fitzhardinge
[not found] ` <4563667B.2060209-TSDbQ3PG+2Y@public.gmane.org>
2006-11-22 6:42 ` Avi Kivity
[not found] ` <4563F158.3060209-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-11-22 9:10 ` Jeremy Fitzhardinge
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox