Re: mips changes for loongson2

[Simon Burge <simonb%NetBSD.org@localhost>]

Manuel Bouyer wrote:

> Hello,
> my work on getting the lemote fuloong working is almost ready to
> be commited (it'll be another evbmips sub-port: evbmips/loongson,
> which should also work on the lebote yeloong, the gdium and probably others).

Good to see it taking shape!

> This requires a few changes to some mips-specific files as in the attached
> patches:
> - bonito.diff:
>   make the 'struct bonito_config' const, so that it can be in kernel
>   text instead of data. This needs some __UNCONST() but I guess
>   it's no big deal.
>   Add a bc_attach_hook to struct bonito_config, to be used as pc_attach_hook
>   if not NULL.
>   Add some LS2-specific register defines (LS2f uses a modified bonito64).

This looks OK.  The lemote fuloong really uses a Bonito or some
derivative thereof?

> - mips.diff:
>   add some LS2F specific defines to cpuregs.h
>   Fix a bug with VM_MAXUSER_ADDRESS and 16k pages (VM_MAXUSER_ADDRESS
>   doens't change with page size ...)
>   Hack some assembly files for LS2 branch prediction bug workaround on
>   j(r) reg, as already discussed here (basically, the assembler needs to
>   be able to use at register when it runs into a j register instruction).
>   LS2-specific tlbmiss handler (ls2 doesn't have a separate handler for
>   64bit addresses)
>   Fixes for some #ifdef MIPS3_LOONGSON2 assembly code (using the wrong
>   register).
>   mips_fixup.c: add code to handle the instructions added by
>   -fix-loongson2-btb
> - common.diff:
>   Hack some assembly files for LS2 branch prediction bug workaround on
>   j(r) reg when used in kernel code by allowing at use where needed.

A comment or two about this below against the diffs.

> - binutils.diff: add fix-loongson2f-btb option to gas, from openbsd.
>   This works around a bug in the branch target buffer of the CPU, which may
>   cause it to fetch instructions at the wrong place and cause the platform
>   to deadlock. This works around the same bug as fix-loongson2f-jump,
>   but in a different way (force a BTB clear through a write to cop0, while
>   fix-loongson2f-jump silently changes the jump address which can cause
>   various problems).

This is someone else's concern :)


> Index: include/cpuregs.h
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/mips/include/cpuregs.h,v
> retrieving revision 1.85
> diff -u -p -u -r1.85 cpuregs.h
> --- include/cpuregs.h 2 Aug 2011 05:12:32 -0000       1.85
> +++ include/cpuregs.h 24 Aug 2011 17:21:23 -0000
> @@ -1,6 +1,22 @@
>  /*   $NetBSD: cpuregs.h,v 1.85 2011/08/02 05:12:32 matt Exp $        */
>  
>  /*
> + * Copyright (c) 2009 Miodrag Vallat.
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +/*

Does a couple of register definitions and macros need a new copyright
section in this file?  There's been _many_ changes to this since
the original without adding copyrights for each new CPU type we've
supported.  I wouldn't include this.

> Index: include/vmparam.h
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/mips/include/vmparam.h,v
> retrieving revision 1.49
> diff -u -p -u -r1.49 vmparam.h
> --- include/vmparam.h 21 Jul 2011 23:03:39 -0000      1.49
> +++ include/vmparam.h 24 Aug 2011 17:21:23 -0000
> @@ -155,7 +155,11 @@
>   */
>  #define VM_MIN_ADDRESS               ((vaddr_t)0x00000000)
>  #ifdef _LP64
> +#if 0
>  #define VM_MAXUSER_ADDRESS   ((vaddr_t) 1L << (4*PGSHIFT-8))
> +#else
> +#define VM_MAXUSER_ADDRESS   ((vaddr_t) 1L << (40))
> +#endif

Can the "40" be described in terms of any existing (or new) constants?
If not, could you please add a comment on how the 40 was derived?  Also,
I'd not leave the "#if 0" block in - cvs will keep the history.

Cheers,
Simon.