| Message ID | 20200512094438.17998-1-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests,v3] Fixes for the umip test | expand |
On 12/05/20 11:44, Thomas Huth wrote: > 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> > --- > v3: Use "sizeof(long)" instead of "2" > > x86/umip.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/x86/umip.c b/x86/umip.c > index 7eee294..37f1ab8 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,8 @@ 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) - > + sizeof(long)]), > [fn]"r"(fn), > [arg]"D"(arg), > [kernel_ds]"i"(KERNEL_DS), > Pushed, thanks. Paolo
diff --git a/x86/umip.c b/x86/umip.c index 7eee294..37f1ab8 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,8 @@ 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) - + sizeof(long)]), [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> --- v3: Use "sizeof(long)" instead of "2" x86/umip.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)