[ECOS] HAL_SavedRegisters (ecos 1_3_1)

[ECOS] HAL_SavedRegisters (ecos 1_3_1)

[Rafael Rodríguez Velilla]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

  I can't find any place where the member msr of the struct
HAL_SavedRegisters is used (I have reviewed only the ARM
implementation). I think that this member should be omitted from the
struct.

  I'd also want to point out that the method load_context of the class
Cyg_HardwareThread seems to me that does the wrong thing. It is
implemented like:
inline void Cyg_HardwareThread ::load_context ()
{
   CYG_ADDRESS dummy_stack_ptr;
   HAL_THREAD_SWITCH_CONTEXT(&dummy_stack, &stack_ptr);
}

I think that HAL_THREAD_LOAD_CONTEXT is the correct function to call
(and dummy_stack_ptr is unnecessary). load_context shouldn't, because of
its definition, save the old context.
(Well, anyway it isn't used anywhere)





--
Rafael Rodríguez Velilla        rrv@tid.es
Telefónica I+D          http://www.tid.es
Telf: +34 - 91 337 4270

[ECOS] Re: HAL_SavedRegisters (ecos 1_3_1)

[Fabrice Gautier]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 546 bytes --]

On Wed, 30 May 2001 19:11:38 +0200
Rafael Rodríguez Velilla <rrv@tid.es> wrote:

>   I can't find any place where the member msr of the struct
> HAL_SavedRegisters is used (I have reviewed only the ARM
> implementation). I think that this member should be omitted from the
> struct.

I think it is used by the gdb stub. This allow gdb to show the value of
this register even if it is never used.

If I remenber coorectly same thing happen in x86 with some segment
registers (cs at least i think) )


-- 
Fabrice Gautier <gautier@email.enstfr>

Re: [ECOS] Re: HAL_SavedRegisters (ecos 1_3_1)

[Rafael Rodríguez Velilla]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

I have followed the code of the GDB stub also, but I didn't find a mention
of that member.



> >   I can't find any place where the member msr of the struct
> > HAL_SavedRegisters is used (I have reviewed only the ARM
> > implementation). I think that this member should be omitted from the
> > struct.
>
> I think it is used by the gdb stub. This allow gdb to show the value of
> this register even if it is never used.
>
> If I remenber coorectly same thing happen in x86 with some segment
> registers (cs at least i think) )
>
> --
> Fabrice Gautier <gautier@email.enstfr>

--
Rafael Rodríguez Velilla        rrv@tid.es
Telefónica I+D          http://www.tid.es
Telf: +34 - 91 337 4270

Re: [ECOS] HAL_SavedRegisters (ecos 1_3_1)

[Jonathan Larmour]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

Rafael Rodríguez Velilla wrote:
> 
>   I can't find any place where the member msr of the struct
> HAL_SavedRegisters is used (I have reviewed only the ARM
> implementation). I think that this member should be omitted from the
> struct.

From what I can see you are right (although Gary should probably confirm).
The only thing is that if we remove it now, it may cause incompatibilities
between a GDB stub in ROM and applications, if built from different
sources, since ARMREG_SIZE in vectors.S will have changed.
 
>   I'd also want to point out that the method load_context of the class
> Cyg_HardwareThread seems to me that does the wrong thing. It is
> implemented like:
> inline void Cyg_HardwareThread ::load_context ()
> {
>    CYG_ADDRESS dummy_stack_ptr;
>    HAL_THREAD_SWITCH_CONTEXT(&dummy_stack, &stack_ptr);
> }
> 
> I think that HAL_THREAD_LOAD_CONTEXT is the correct function to call
> (and dummy_stack_ptr is unnecessary). load_context shouldn't, because of
> its definition, save the old context.

Again I think you're right. Nick, do you agree?

Jifl
-- 
Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062
Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine
<advert> Come to the Red Hat TechWorld open source conference in Brussels!
Keynotes, techie talks and exhibitions    http://www.redhat-techworld.com/

Re: [ECOS] HAL_SavedRegisters (ecos 1_3_1)

[Nick Garnett]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]

Jonathan Larmour <jlarmour@redhat.com> writes:

> Rafael Rodríguez Velilla wrote:

> >   I'd also want to point out that the method load_context of the class
> > Cyg_HardwareThread seems to me that does the wrong thing. It is
> > implemented like:
> > inline void Cyg_HardwareThread ::load_context ()
> > {
> >    CYG_ADDRESS dummy_stack_ptr;
> >    HAL_THREAD_SWITCH_CONTEXT(&dummy_stack, &stack_ptr);
> > }
> > 
> > I think that HAL_THREAD_LOAD_CONTEXT is the correct function to call
> > (and dummy_stack_ptr is unnecessary). load_context shouldn't, because of
> > its definition, save the old context.
> 
> Again I think you're right. Nick, do you agree?
> 

We should probably get rid of the whole function. It is no longer
necessary and its presence might lure the unwary into actually calling
it, with dire consequences.

-- 
Nick Garnett, eCos Kernel Architect
Red Hat, Cambridge, UK

Re: [ECOS] HAL_SavedRegisters (ecos 1_3_1)

[Gary Thomas]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 852 bytes --]

On 31-May-2001 Jonathan Larmour wrote:
> Rafael Rodríguez Velilla wrote:
>> 
>>   I can't find any place where the member msr of the struct
>> HAL_SavedRegisters is used (I have reviewed only the ARM
>> implementation). I think that this member should be omitted from the
>> struct.
> 
>From what I can see you are right (although Gary should probably confirm).
> The only thing is that if we remove it now, it may cause incompatibilities
> between a GDB stub in ROM and applications, if built from different
> sources, since ARMREG_SIZE in vectors.S will have changed.

I'm pretty sure that I defined that register _just in case_ it was important.
It might still be with some future version of the ARM chip.

I'd rather not remove it, both to retain pure compatability and for stability
reasons.  I also don't see much benefit from saving 4 bytes.

Re: [ECOS] HAL_SavedRegisters (ecos 1_3_1)

[Rafael Rodríguez Velilla]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

Gary Thomas wrote:

> On 31-May-2001 Jonathan Larmour wrote:
> > Rafael Rodríguez Velilla wrote:
> >>
> >>   I can't find any place where the member msr of the struct
> >> HAL_SavedRegisters is used (I have reviewed only the ARM
> >> implementation). I think that this member should be omitted from the
> >> struct.
> >
> >>From what I can see you are right (although Gary should probably confirm).
> > The only thing is that if we remove it now, it may cause incompatibilities
> > between a GDB stub in ROM and applications, if built from different
> > sources, since ARMREG_SIZE in vectors.S will have changed.
>
> I'm pretty sure that I defined that register _just in case_ it was important.
> It might still be with some future version of the ARM chip.
>
> I'd rather not remove it, both to retain pure compatability and for stability
> reasons.  I also don't see much benefit from saving 4 bytes.

  What about using a cdl option to let the user decide if he wants that kind of
backward compatibility?


--
Rafael Rodríguez Velilla        rrv@tid.es
Telefónica I+D          http://www.tid.es
Telf: +34 - 91 337 4270