Re: [Qemu-devel] [PATCH 4/6] simply cpu_exec further

["Glauber Costa" <glommer@gmail.com>]

On Tue, May 27, 2008 at 1:18 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On 5/27/08, Glauber Costa <gcosta@redhat.com> wrote:
>> We change interrupt functions so they have the same
>>  signature, getting only an env parameter. When necessary,
>>  some more attributed were added to the relevant CPUState to
>>  make it possible.
>
>>  +void arch_handle_interrupt_request(CPUState *env);
>
> I'd use cpu_handle_interrupt_request().

fair enough. It does not follow my own naming scheme.

>
>>  --- a/target-sparc/op_helper.c
>>  +++ b/target-sparc/op_helper.c
>>  @@ -2689,8 +2689,9 @@ static const char * const excp_names[0x50] = {
>>   };
>>   #endif
>>
>>  -void do_interrupt(int intno)
>>  +void do_interrupt(CPUState *cpu)
>
> env1 would be better.
>
>>   {
>>  +    int intno = cpu->exception_index;
>>   #ifdef DEBUG_PCALL
>>      if (loglevel & CPU_LOG_INT) {
>>          static int count;
>>  @@ -2803,9 +2804,10 @@ static const char * const excp_names[0x80] = {
>>   };
>>   #endif
>>
>>  -void do_interrupt(int intno)
>>  +void do_interrupt(CPUState *env)
>
> Same here. Otherwise you shadow the AREG0 env.
>
>>  +void arch_handle_interrupt_request(CPUState *env)
>
> And here.
>
> I think this function and do_interrupt should belong to helper.c, they
> are not called from ops.  I'll do some rearranging. After that my env
> comment does not matter.
Agreed. I had problems compiling this in helper.c, but if you can move
the code so to better acomodate those, I'd happily change
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."