[ECOS] MIPS R3000 patch for MIPS arch

[ECOS] MIPS R3000 patch for MIPS arch

[sun409]

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

The attach is the patch file for MIPS architecture to
make the source codes in hal/mips/arch/ directory comply
with MIPS I ISA.

There are mainly two kinds of modification:
1. Add a delay slot after lw insn.
2. Change non-MIPS I insn. to equiv. MIPS I insn.

I hope you in Cygnus could accept this patch.
And, any further modification to the hal/mips/arch/
will not use instructions other than MIPS I.

Regards,

Tim Ouyang
_________________________________________________________
 pchome §K¶O¹q¤l«H½c¡A¥Ó½Ð½Ð¦Ü: http://www.pchome.com.tw 

RE: [ECOS] MIPS R3000 patch for MIPS arch

[Jurica Baricevic]

> Tim Ouyang wrote:
>
> The attach is the patch file for MIPS architecture to
> make the source codes in hal/mips/arch/ directory comply
> with MIPS I ISA.
>
> There are mainly two kinds of modification:
> 1. Add a delay slot after lw insn.
> 2. Change non-MIPS I insn. to equiv. MIPS I insn.
>
> I hope you in Cygnus could accept this patch.
> And, any further modification to the hal/mips/arch/
> will not use instructions other than MIPS I.
>


Thank you. A few days ago I tested eCos 1.3.1 with modifications very
similar to yours on an R3041 board and I can say that everything worked
well. Since 'load delay slot' seemed as the main problem in porting to
R3000, I really hope that Cygnus could accept your patch. Also, I believe
that distinguishing between MIPS processors in the mips/arch directory
should be oriented to MIPS ISA levels (hopefully in next eCos
version/revision :-) ). Namely, '#ifdef CYG_HAL_MIPS_R3900' and similar
statements should be avoided if possible; and something like '#ifdef
CYG_HAL_MIPS_ISA1' should be put on right places instead.


I have a question with regard to the patch:
Patch in 'packages/hal/mips/arch/current/include/hal_intr.h' (macro
HAL_RESTORE_INTERRUPTS) looks as a bug fix for all MIPS ISA levels. Am I
right?


Best regards,
Jura.

Re: [ECOS] MIPS R3000 patch for MIPS arch

[Jonathan Larmour]

Jurica Baricevic wrote:
> 
> Thank you. A few days ago I tested eCos 1.3.1 with modifications very
> similar to yours on an R3041 board and I can say that everything worked
> well. Since 'load delay slot' seemed as the main problem in porting to
> R3000, I really hope that Cygnus could accept your patch.

Me too :-).

> Also, I believe
> that distinguishing between MIPS processors in the mips/arch directory
> should be oriented to MIPS ISA levels (hopefully in next eCos
> version/revision :-) ). 

Why wait :-). It would benefit anonymous CVS users immediately.

> Namely, '#ifdef CYG_HAL_MIPS_R3900' and similar
> statements should be avoided if possible; and something like '#ifdef
> CYG_HAL_MIPS_ISA1' should be put on right places instead.

I agree, and in fact I'd be happy to take this patch with such
improvements. Tim, I think it would be more generic for each variant HAL to
define the ISA level supported in the CDL, e.g. for tx39:

cdl_option CYGHWR_HAL_MIPS_ISA {
    display     "MIPS ISA level supported by CPU"
    calculated  2
}

I believe ISA 2 is correct for the tx39 since it supports branch likely
instructions. For the vr4300 it would be:

cdl_option CYGHWR_HAL_MIPS_ISA {
    display     "MIPS ISA level supported by CPU"
    calculated  3
}

Then at the top of vectors.S you could write:
#if CYGHWR_HAL_MIPS_ISA == 1
#define LWNOP nop
#else
#define LWNOP
#endif

Then instead of adding the explicit nops in vectors.S for all ISAs, use
this LWNOP macro, which means the (admittedly small) overhead will only
happen for CPUs with ISA 1.

Also you don't need to add "sun409@pchome.com.tw added" every nop line :-).
Perhaps just say "MIPS ISA 1 lw delay slot"

Similarly, for your change to hal_cpu_eret in arch.inc, we can instead
have:

        # Return from exception.
#ifdef CYGHWR_HAL_MIPS_ISA == 1
       .macro  hal_cpu_eret pc,sr
       mtc0    \sr,status                      # Load status register
       nop
       jr      \pc                             # jump back to interrupted
code
       rfe                                     # restore state (delay
slot)    
       .endm
#elif CYGHWR_HAL_MIPS_ISA == 2
        .macro  hal_cpu_eret pc,sr
        mtc0    \sr,status                      # Load status register
        nop
        nop
        nop
        sync                                    # settle things down
        jr      \pc                             # jump back to interrupted
code
        rfe                                     # restore state (delay
slot)
        .endm
#else   # ISA >= 3
        .macro  hal_cpu_eret pc,sr
        .set mips3
[etc.]

If you can make these changes, I'll check it in.

> I have a question with regard to the patch:
> Patch in 'packages/hal/mips/arch/current/include/hal_intr.h' (macro
> HAL_RESTORE_INTERRUPTS) looks as a bug fix for all MIPS ISA levels. Am I
> right?

Yes. Although it doesn't happen to cause any problems in the existing code
because the macro is only used in a limited way. But obviously the fix is
welcome.

Jifl
-- 
Red Hat, 35 Cambridge Place, Cambridge, UK. CB2 1NS  Tel: +44 (1223) 728762
"Plan to be spontaneous tomorrow."  ||  These opinions are all my own fault

Re: [ECOS] MIPS R3000 patch for MIPS arch

[Nick Garnett]

Jonathan Larmour <jlarmour@redhat.co.uk> writes:

> Jurica Baricevic wrote:
> > 

> > Namely, '#ifdef CYG_HAL_MIPS_R3900' and similar
> > statements should be avoided if possible; and something like '#ifdef
> > CYG_HAL_MIPS_ISA1' should be put on right places instead.
> 
> I agree, and in fact I'd be happy to take this patch with such
> improvements. Tim, I think it would be more generic for each variant HAL to
> define the ISA level supported in the CDL, e.g. for tx39:


Bear in mind that most real MIPS implementations are actually a
mixture of ISA levels, very few adhere strictly to just one. The TX39,
for example is mostly ISA2 with some ISA3 instructions added (cache
instructions for example). So naive use of straight ISA levels will
not allow an accurate description of the instruction set of any actual
implementation.

There really needs to be a separate configuration option for each
place where there is a choice of instructions, and to control these
individually. That way the TX39 can have ISA2 style interrupt returns
with ISA3 cache control.

-- 
Nick Garnett
Cygnus Solutions, a Red Hat Company
Cambridge, UK

Re: [ECOS] MIPS R3000 patch for MIPS arch

[Nick Garnett]

Jonathan Larmour <jlarmour@redhat.co.uk> writes:

> Jurica Baricevic wrote:
> > 

> > Namely, '#ifdef CYG_HAL_MIPS_R3900' and similar
> > statements should be avoided if possible; and something like '#ifdef
> > CYG_HAL_MIPS_ISA1' should be put on right places instead.
> 
> I agree, and in fact I'd be happy to take this patch with such
> improvements. Tim, I think it would be more generic for each variant HAL to
> define the ISA level supported in the CDL, e.g. for tx39:


Bear in mind that most real MIPS implementations are actually a
mixture of ISA levels, very few adhere strictly to just one. The TX39,
for example is mostly ISA2 with some ISA3 instructions added (cache
instructions for example). So naive use of straight ISA levels will
not allow an accurate description of the instruction set of any actual
implementation.

There really needs to be a separate configuration option for each
place where there is a choice of instructions, and to control these
individually. That way the TX39 can have ISA2 style interrupt returns
with ISA3 cache control.

-- 
Nick Garnett
Cygnus Solutions, a Red Hat Company
Cambridge, UK