[PATCH] kvm-unit-tests: Fix GCC's 4.8 labels as values for nVMX tests.

[PATCH] kvm-unit-tests: Fix GCC's 4.8 labels as values for nVMX tests.

[Marius Vlad]

Commit 3b1274463fa8d074dd3bc77efe25b59a4ddd491e uses GCCs extension
labels as values to handle exceptions, but GCC 4.8 ``mistakingly''
uses the next body function as a jump label, for functions which
do not return. Fixed by returning a int value for those functions.

See http://thread.gmane.org/gmane.comp.emulators.kvm.devel/119186

Signed-off-by: Marius Vlad <mv@sec.uni-passau.de>
---
 x86/vmx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index fe950e6..0c895af 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -548,7 +548,7 @@ static void exception_handler(struct ex_regs *regs)
 	regs->rip = (u64)exception_return;
 }
 
-static int test_for_exception(unsigned int ex, void (*func)(void))
+static int test_for_exception(unsigned int ex, int (*func)(void))
 {
 	handle_exception(ex, exception_handler);
 	exception = false;
@@ -557,23 +557,23 @@ static int test_for_exception(unsigned int ex, void (*func)(void))
 	return exception;
 }
 
-static void do_vmxon_off(void)
+static int do_vmxon_off(void)
 {
 	exception_return = &&resume;
 	barrier();
 	vmx_on();
 	vmx_off();
 resume:
-	return;
+	return 0;
 }
 
-static void do_write_feature_control(void)
+static int do_write_feature_control(void)
 {
 	exception_return = &&resume;
 	barrier();
 	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
 resume:
-	return;
+	return 0;
 }
 
 static int test_vmx_feature_control(void)
-- 
1.8.5.3

Re: [PATCH] kvm-unit-tests: Fix GCC's 4.8 labels as values for nVMX tests.

[Jan Kiszka]

On 2014-02-24 16:25, Marius Vlad wrote:
> Commit 3b1274463fa8d074dd3bc77efe25b59a4ddd491e uses GCCs extension
> labels as values to handle exceptions, but GCC 4.8 ``mistakingly''
> uses the next body function as a jump label, for functions which
> do not return. Fixed by returning a int value for those functions.
> 
> See http://thread.gmane.org/gmane.comp.emulators.kvm.devel/119186
> 
> Signed-off-by: Marius Vlad <mv@sec.uni-passau.de>
> ---
>  x86/vmx.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/x86/vmx.c b/x86/vmx.c
> index fe950e6..0c895af 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -548,7 +548,7 @@ static void exception_handler(struct ex_regs *regs)
>  	regs->rip = (u64)exception_return;
>  }
>  
> -static int test_for_exception(unsigned int ex, void (*func)(void))
> +static int test_for_exception(unsigned int ex, int (*func)(void))
>  {
>  	handle_exception(ex, exception_handler);
>  	exception = false;
> @@ -557,23 +557,23 @@ static int test_for_exception(unsigned int ex, void (*func)(void))
>  	return exception;
>  }
>  
> -static void do_vmxon_off(void)
> +static int do_vmxon_off(void)
>  {
>  	exception_return = &&resume;
>  	barrier();
>  	vmx_on();
>  	vmx_off();
>  resume:
> -	return;
> +	return 0;
>  }
>  
> -static void do_write_feature_control(void)
> +static int do_write_feature_control(void)
>  {
>  	exception_return = &&resume;
>  	barrier();
>  	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
>  resume:
> -	return;
> +	return 0;
>  }
>  
>  static int test_vmx_feature_control(void)
> 

Argh, getting old. I remembered that issue but forgot that I already had
a fix for this queued:

http://thread.gmane.org/gmane.comp.emulators.kvm.devel/117866

I don't mind which version to pick, but maybe Paolo has mine already in
his queue.

Sorry for the duplicate work.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH] kvm-unit-tests: Fix GCC's 4.8 labels as values for nVMX tests.

[Marius Vlad]

[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]

On Mon, Feb 24, 2014 at 04:58:45PM +0100, Jan Kiszka wrote:
> On 2014-02-24 16:25, Marius Vlad wrote:
> > Commit 3b1274463fa8d074dd3bc77efe25b59a4ddd491e uses GCCs extension
> > labels as values to handle exceptions, but GCC 4.8 ``mistakingly''
> > uses the next body function as a jump label, for functions which
> > do not return. Fixed by returning a int value for those functions.
> > 
> > See http://thread.gmane.org/gmane.comp.emulators.kvm.devel/119186
> > 
> > Signed-off-by: Marius Vlad <mv@sec.uni-passau.de>
> > ---
> >  x86/vmx.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/x86/vmx.c b/x86/vmx.c
> > index fe950e6..0c895af 100644
> > --- a/x86/vmx.c
> > +++ b/x86/vmx.c
> > @@ -548,7 +548,7 @@ static void exception_handler(struct ex_regs *regs)
> >  	regs->rip = (u64)exception_return;
> >  }
> >  
> > -static int test_for_exception(unsigned int ex, void (*func)(void))
> > +static int test_for_exception(unsigned int ex, int (*func)(void))
> >  {
> >  	handle_exception(ex, exception_handler);
> >  	exception = false;
> > @@ -557,23 +557,23 @@ static int test_for_exception(unsigned int ex, void (*func)(void))
> >  	return exception;
> >  }
> >  
> > -static void do_vmxon_off(void)
> > +static int do_vmxon_off(void)
> >  {
> >  	exception_return = &&resume;
> >  	barrier();
> >  	vmx_on();
> >  	vmx_off();
> >  resume:
> > -	return;
> > +	return 0;
> >  }
> >  
> > -static void do_write_feature_control(void)
> > +static int do_write_feature_control(void)
> >  {
> >  	exception_return = &&resume;
> >  	barrier();
> >  	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
> >  resume:
> > -	return;
> > +	return 0;
> >  }
> >  
> >  static int test_vmx_feature_control(void)
> > 
> 

 I see, no biggie, my fault for not looking on the list before going any deeper :-).

> Argh, getting old. I remembered that issue but forgot that I already had
> a fix for this queued:
> 
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/117866
> 
> I don't mind which version to pick, but maybe Paolo has mine already in
> his queue.
> 
> Sorry for the duplicate work.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SES-DE
> Corporate Competence Center Embedded Linux
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] kvm-unit-tests: Fix GCC's 4.8 labels as values for nVMX tests.

[Paolo Bonzini]

Il 24/02/2014 16:58, Jan Kiszka ha scritto:
> On 2014-02-24 16:25, Marius Vlad wrote:
>> Commit 3b1274463fa8d074dd3bc77efe25b59a4ddd491e uses GCCs extension
>> labels as values to handle exceptions, but GCC 4.8 ``mistakingly''
>> uses the next body function as a jump label, for functions which
>> do not return. Fixed by returning a int value for those functions.
>>
>> See http://thread.gmane.org/gmane.comp.emulators.kvm.devel/119186
>>
>> Signed-off-by: Marius Vlad <mv@sec.uni-passau.de>
>> ---
>>  x86/vmx.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/x86/vmx.c b/x86/vmx.c
>> index fe950e6..0c895af 100644
>> --- a/x86/vmx.c
>> +++ b/x86/vmx.c
>> @@ -548,7 +548,7 @@ static void exception_handler(struct ex_regs *regs)
>>  	regs->rip = (u64)exception_return;
>>  }
>>
>> -static int test_for_exception(unsigned int ex, void (*func)(void))
>> +static int test_for_exception(unsigned int ex, int (*func)(void))
>>  {
>>  	handle_exception(ex, exception_handler);
>>  	exception = false;
>> @@ -557,23 +557,23 @@ static int test_for_exception(unsigned int ex, void (*func)(void))
>>  	return exception;
>>  }
>>
>> -static void do_vmxon_off(void)
>> +static int do_vmxon_off(void)
>>  {
>>  	exception_return = &&resume;
>>  	barrier();
>>  	vmx_on();
>>  	vmx_off();
>>  resume:
>> -	return;
>> +	return 0;
>>  }
>>
>> -static void do_write_feature_control(void)
>> +static int do_write_feature_control(void)
>>  {
>>  	exception_return = &&resume;
>>  	barrier();
>>  	wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
>>  resume:
>> -	return;
>> +	return 0;
>>  }
>>
>>  static int test_vmx_feature_control(void)
>>
>
> Argh, getting old. I remembered that issue but forgot that I already had
> a fix for this queued:
>
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/117866
>
> I don't mind which version to pick, but maybe Paolo has mine already in
> his queue.

Yeah (I hadn't, but now I have).  According to GCC developers this is 
invalid code.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28581

Perhaps we should rewrite this using something more similar to "normal" 
C setjmp/longjmp.

Paolo

Re: [PATCH] kvm-unit-tests: Fix GCC's 4.8 labels as values for nVMX tests.

[Jan Kiszka]

On 2014-02-25 15:26, Paolo Bonzini wrote:
> Il 24/02/2014 16:58, Jan Kiszka ha scritto:
>> On 2014-02-24 16:25, Marius Vlad wrote:
>>> Commit 3b1274463fa8d074dd3bc77efe25b59a4ddd491e uses GCCs extension
>>> labels as values to handle exceptions, but GCC 4.8 ``mistakingly''
>>> uses the next body function as a jump label, for functions which
>>> do not return. Fixed by returning a int value for those functions.
>>>
>>> See http://thread.gmane.org/gmane.comp.emulators.kvm.devel/119186
>>>
>>> Signed-off-by: Marius Vlad <mv@sec.uni-passau.de>
>>> ---
>>>  x86/vmx.c | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/x86/vmx.c b/x86/vmx.c
>>> index fe950e6..0c895af 100644
>>> --- a/x86/vmx.c
>>> +++ b/x86/vmx.c
>>> @@ -548,7 +548,7 @@ static void exception_handler(struct ex_regs *regs)
>>>      regs->rip = (u64)exception_return;
>>>  }
>>>
>>> -static int test_for_exception(unsigned int ex, void (*func)(void))
>>> +static int test_for_exception(unsigned int ex, int (*func)(void))
>>>  {
>>>      handle_exception(ex, exception_handler);
>>>      exception = false;
>>> @@ -557,23 +557,23 @@ static int test_for_exception(unsigned int ex,
>>> void (*func)(void))
>>>      return exception;
>>>  }
>>>
>>> -static void do_vmxon_off(void)
>>> +static int do_vmxon_off(void)
>>>  {
>>>      exception_return = &&resume;
>>>      barrier();
>>>      vmx_on();
>>>      vmx_off();
>>>  resume:
>>> -    return;
>>> +    return 0;
>>>  }
>>>
>>> -static void do_write_feature_control(void)
>>> +static int do_write_feature_control(void)
>>>  {
>>>      exception_return = &&resume;
>>>      barrier();
>>>      wrmsr(MSR_IA32_FEATURE_CONTROL, 0);
>>>  resume:
>>> -    return;
>>> +    return 0;
>>>  }
>>>
>>>  static int test_vmx_feature_control(void)
>>>
>>
>> Argh, getting old. I remembered that issue but forgot that I already had
>> a fix for this queued:
>>
>> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/117866
>>
>> I don't mind which version to pick, but maybe Paolo has mine already in
>> his queue.
> 
> Yeah (I hadn't, but now I have).  According to GCC developers this is
> invalid code.
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28581

Oh - how nicely intuitive.

> 
> Perhaps we should rewrite this using something more similar to "normal"
> C setjmp/longjmp.

Hmm, maybe. If barrier workaround wouldn't work already...

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux