In article <4368110F.5020905@tadpole.com>
garrett_damore@tadpole.com wrote:

> To set up a full TLB the MD code will have to call this routine once 
> each for PA0 and PA1.

Ok, it seems fine.

> +++ sys/arch/mips/mips/mips3_wired_map.c	2 Nov 2005 01:06:10 -0000

As I wrote before, this can be arch/mips/mips/wired_map.c.
I'll rename arc/wired_map.c to wired_map_machdep.c or something.

> +#include <mips/wired_map.h>

Maybe we should have <machine/wired_map.h> for MD definitions,
which includes <mips/wired_map.h> like <machine/vmparam.h>.

> +static struct wired_map_entry {
> +	paddr_t	pa0;
> +	paddr_t	pa1;
> +	vaddr_t	va;
> +	vsize_t	pgmask;
> +} wired_map[MIPS3_WIRED_ENTRIES];

This struct wired_map_entry and wired_map[] variable should be
exported for MD functions.

> +static boolean_t mips3_wire_down_page(vaddr_t va, paddr_t pa);

This should be removed (exported).

> +static int	nwired;

nwired also should be exported, though maybe it's better
to rename it something like mips3_nwired_page.

> +boolean_t
> +mips3_wire_down_page(vaddr_t va, paddr_t pa, vsize_t pgsz)

IMHO, "wire_down" seems a bit strange for this operation.
It reminds me "pin down" memory (RAM) to avoid pageout.
How about "mips3_wired_enter_page()" like pmap_enter(9)?

> +	if ((va & (pgsz - 1)) || (pa & (pgsz - 1)))
> +		panic("mips3_wire_down_page: not aligned");

This check could be wrapped by #ifdef DIAGNOSTIC.

> +			return 0;
 :
> +	return 1;

It's better to use TRUE and FALSE rather than 1 and 0?

> +boolean_t
> +mips3_wire_down(vaddr_t va, paddr_t pa, vsize_t size)

How about mips3_wired_enter() for this?

> Index: sys/arch/mips/include/wired_map.h
 :
> +/*
> + * Certain machines have peripheral busses which are only accessible
> + * using the TLB.
 :
> + * Note also that at the moment this is not supported on the MIPS-I
> + * ISA (but it shouldn't need it anyway.)
> + */

Isn't it better to move these comments to (mips3_)wired_map.c?

> +#define	MIPS3_SIZE_TO_PG_MASK(x)	(((x * 2) - 1) & ~0x1fff)

This is also defined in <mips/mips3_pte.h> later?

> +#ifndef	MIPS3_WIRED_ENTRIES
> +#define MIPS3_WIRED_ENTRIES	8	/* upper limit */
> +#endif	/* MIPS3_WIRED_ENTRIES */

How about "MIPS3_NWIRED_ENTRY" like VM_NFREELIST in vmparam.h?

> +/*
> + * Wire down a mapping from a virtual to physical address.  The size
> + * of the region must be a multiple of MIPS3_WIRED_ENTRY_SIZE, with
> + * matching alignment.
> + */

> +/*
> + * Lower layer API, to supply an explicit page size.  It only wires a
> + * single page at a time.
> + */

These function descriptions should be in (mips3_)wired_map.c.


Anyway, I'll try to adapt arc port to your patch shortly. Thanks!
---
Izumi Tsutsui