| Message ID | 20200210143514.5347-1-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,v2] Fixes for the umip test | expand |
On 10/02/20 15:35, Thomas Huth wrote: > #define GP_ASM(stmt, in, clobber) \ > - asm ("mov" W " $1f, %[expected_rip]\n\t" \ > + asm volatile ( \ > + "mov" W " $1f, %[expected_rip]\n\t" \ > "movl $2f-1f, %[skip_count]\n\t" \ > "1: " stmt "\n\t" \ > "2: " \ > @@ -159,7 +160,7 @@ static int do_ring3(void (*fn)(const char *), const char *arg) > : [ret] "=&a" (ret) > : [user_ds] "i" (USER_DS), > [user_cs] "i" (USER_CS), > - [user_stack_top]"m"(user_stack[sizeof user_stack]), > + [user_stack_top]"m"(user_stack[sizeof(user_stack) - 2]), This should be "- sizeof(long)" in order to keep the stack aligned. I can fix this when I apply. Paolo > [fn]"r"(fn), > [arg]"D"(arg), > [kernel_ds]"i"(KERNEL_DS), >
On 10/02/2020 15.56, Paolo Bonzini wrote: > On 10/02/20 15:35, Thomas Huth wrote: >> #define GP_ASM(stmt, in, clobber) \ >> - asm ("mov" W " $1f, %[expected_rip]\n\t" \ >> + asm volatile ( \ >> + "mov" W " $1f, %[expected_rip]\n\t" \ >> "movl $2f-1f, %[skip_count]\n\t" \ >> "1: " stmt "\n\t" \ >> "2: " \ >> @@ -159,7 +160,7 @@ static int do_ring3(void (*fn)(const char *), const char *arg) >> : [ret] "=&a" (ret) >> : [user_ds] "i" (USER_DS), >> [user_cs] "i" (USER_CS), >> - [user_stack_top]"m"(user_stack[sizeof user_stack]), >> + [user_stack_top]"m"(user_stack[sizeof(user_stack) - 2]), > > This should be "- sizeof(long)" in order to keep the stack aligned. > > I can fix this when I apply. Thanks! Thomas
diff --git a/x86/umip.c b/x86/umip.c index 7eee294..339bea4 100644 --- a/x86/umip.c +++ b/x86/umip.c @@ -22,7 +22,8 @@ static void gp_handler(struct ex_regs *regs) #define GP_ASM(stmt, in, clobber) \ - asm ("mov" W " $1f, %[expected_rip]\n\t" \ + asm volatile ( \ + "mov" W " $1f, %[expected_rip]\n\t" \ "movl $2f-1f, %[skip_count]\n\t" \ "1: " stmt "\n\t" \ "2: " \ @@ -159,7 +160,7 @@ static int do_ring3(void (*fn)(const char *), const char *arg) : [ret] "=&a" (ret) : [user_ds] "i" (USER_DS), [user_cs] "i" (USER_CS), - [user_stack_top]"m"(user_stack[sizeof user_stack]), + [user_stack_top]"m"(user_stack[sizeof(user_stack) - 2]), [fn]"r"(fn), [arg]"D"(arg), [kernel_ds]"i"(KERNEL_DS),
When compiling umip.c with -O2 instead of -O1, there are currently two problems. First, the compiler complains: x86/umip.c: In function ‘do_ring3’: x86/umip.c:162:37: error: array subscript 4096 is above array bounds of ‘unsigned char[4096]’ [-Werror=array-bounds] [user_stack_top]"m"(user_stack[sizeof user_stack]), ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ This can be fixed by initializing the stack to point to one of the last bytes of the array instead. The second problem is that some tests are failing - and this is due to the fact that the GP_ASM macro uses inline asm without the "volatile" keyword - so that the compiler reorders this code in certain cases where it should not. Fix it by adding "volatile" here. Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: Add parentheses and fix doubled whitespace as requested by Vitaly x86/umip.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)