Subject: Re: port-mips/31915: provide centralized wired_map logic
To: None <tsutsui@netbsd.org, gnats-admin@netbsd.org,>
From: Garrett D'Amore <garrett_damore@tadpole.com>
List: netbsd-bugs
Date: 10/27/2005 18:28:02
The following reply was made to PR port-mips/31915; it has been noted by GNATS.
From: "Garrett D'Amore" <garrett_damore@tadpole.com>
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Cc: gnats-bugs@NetBSD.org, port-mips@NetBSD.org
Subject: Re: port-mips/31915: provide centralized wired_map logic
Date: Thu, 27 Oct 2005 11:27:06 -0700
I've thought some more about this. I really think, pretty strongly,
that it is wrong for the wired_map logic to manage its own extents.
Here's an example of the problem:
Imagine two PCI devices:
PCI device A needs 16KB memory, and 1KB I/O space.
PCI device B needs 16KB memory.
These have to map to very physical addresses, because I/O space is at a
totally different physical region. Therefore you must allocate these in
16MB (or whatever the page size is) chunks to get separate mappings.
In this case, to get the right constraints, you have to request the
extent manager allocate in increments of a whole page. This means that
either the PCI layer has to also keep track of 16MB chunks and do its
own extent management within those (and try not to allocate a wired page
entry otherwise), or you wind up with a lot of wastage (each mapping
consuming a full 16MB of VA in the simple case, and a full TLB page in
any case.)
A much cleaner, IMO, solution, is to foist the problem of managing VA
space on to the upper layer (PCI in this case). Then it can allocate
separate extents for each of the physical/virtual spaces, and you can
wind up with much simpler code and much more efficient use of the TLB
entries. Granted this approach can leave big gaps in the VA space for
different physical spaces (i.e. a big window for PCI memory, vs. a big
window for PCMCIA, etc.) But the machine dependent layer has knowledge
of how the VA space is carved up and can subdivide the spaces as finely
as necessary to suit its own needs.
If you allocate a large VA space (e.g. 256 MB VA space for PCI memory)
in your MD code, then you will wind up using a full TLB entry regardless
of whether or not you need it (no sharing of the LO0 and LO1), but if
you don't like this, then just limit the VA window to 16MB in the MD code.
I will be submitting updated patches which use only 16MB pages to allow
for separation of the LO0 and LO1 pages. Again, this only will gain you
anything if you have VA spaces of less than 32 MB. (E.g. PCI I/O and
PCI config space can split a 32MB region of VA space.)
-- Garrett
Izumi Tsutsui wrote:
>In article <20051025205402.7B93A63B8C2@narn.netbsd.org>
>garrett_damore@tadpole.com wrote:
>
>
>
>> Here are the diffs. Again, I've not conditionalized the inclusion of
>> mips3_wired_map.c, but if folks feel strongly enough about it I can
>> certainly do so.
>>
>>
>
>IMO, currently only arc port uses wired map, so it might be better to
>have some proper option (like options MIPS3_WIRED_MAP etc.).
>
>
>
>> Also, the ARC port could probably make use of my logic in
>> mips3_wired_map.c, but I wanted to minimize change (and I don't have an
>> ARC system to test against, anyway, so all I can do is verify the compile.)
>>
>>
>
>I think the MI wired map function should also handle independent
>(non-contiguous) PA ranges for TLB_LO0 and TLB_LO1.
>arc port requires it to map PCI memory and I/O space at the early
>bootstrap stage, and it could save wired TLB entries in other case.
>
>I also have some more quick comments:
>
>- VA region for wired map should be defined in machine dependent
> <machine/vmparam.h> and it should be managed by extent(9) etc.
>- pagemask for wired maps should be defined in machine dependent headers,
> or maybe it should be passed from callers.
>
>Currently I use the attached wired_map.[ch] for ews4800mips ports,
>though it lacks some sanity checks and unmap (or remap) functions.
>
>Maybe we should discuss more about APIs for these wired map functions.
>---
>Izumi Tsutsui
>
>
>---
>/* $NetBSD$ */
>
>/*-
> * Copyright (C) 2005 Izumi Tsutsui.
> * Copyright (C) 2000 Shuichiro URATA. All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions
> * are met:
> * 1. Redistributions of source code must retain the above copyright
> * notice, this list of conditions and the following disclaimer.
> * 2. Redistributions in binary form must reproduce the above copyright
> * notice, this list of conditions and the following disclaimer in the
> * documentation and/or other materials provided with the distribution.
> * 3. The name of the author may not be used to endorse or promote products
> * derived from this software without specific prior written permission.
> *
> * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
>#include <sys/cdefs.h>
>__KERNEL_RCSID(0, "$NetBSD$");
>
>#include <sys/param.h>
>#include <sys/systm.h>
>#include <sys/malloc.h>
>#include <sys/extent.h>
>
>#include <uvm/uvm_extern.h>
>
>#include <mips/pte.h>
>
>#include <machine/locore.h>
>#include <machine/vmparam.h>
>
>#include <ews4800mips/ews4800mips/wired_map.h>
>
>#define EWS4800MIPS_TLB_WIRED_ENTRIES 8 /* upper limit */
>
>static int ews4800mips_enter_wired(vaddr_t va, paddr_t pa0, paddr_t pa1,
> uint32_t pg_size);
>
>static struct wired_map_entry {
> paddr_t pa0;
> paddr_t pa1;
> vsize_t size;
> vaddr_t va;
>} wired_map[EWS4800MIPS_TLB_WIRED_ENTRIES];
>
>static int nwired;
>static struct extent *wired_map_ex;
>static long wired_map_ex_storage[EXTENT_FIXED_STORAGE_SIZE(8) / sizeof(long)];
>
>void
>ews4800mips_init_wired_map(void)
>{
>
> nwired = 0;
> wired_map_ex = extent_create("wired_map",
> VM_MIN_WIRED_MAP_ADDRESS, VM_MAX_WIRED_MAP_ADDRESS, M_DEVBUF,
> (void *)wired_map_ex_storage, sizeof(wired_map_ex_storage),
> EX_NOWAIT);
> if (wired_map_ex == NULL)
> panic("ews4800mips_init_wired_map: can't create extent");
>}
>
>static int
>ews4800mips_enter_wired(vaddr_t va, paddr_t pa0, paddr_t pa1, uint32_t pg_size)
>{
> struct tlb tlb;
> psize_t size;
>
> if (nwired >= EWS4800MIPS_TLB_WIRED_ENTRIES) {
>#ifdef DIAGNOSTIC
> printf("ews4800mips_enter_wired: wired entry exausted.\n");
>#endif
> return ENOMEM;
> }
>
> size = MIPS3_PG_SIZE_MASK_TO_SIZE(pg_size);
> va = (va + (size - 1)) & ~(size - 1);
>
> wired_map[nwired].va = va;
> wired_map[nwired].pa0 = pa0;
> wired_map[nwired].pa1 = pa1;
> wired_map[nwired].size = size;
>
> /* Allocate new wired entry */
> mips3_cp0_wired_write(MIPS3_TLB_WIRED_UPAGES + nwired + 1);
>
> /* Map to it */
> tlb.tlb_mask = pg_size;
> tlb.tlb_hi = mips3_vad_to_vpn(va);
> if (pa0 == 0)
> tlb.tlb_lo0 = MIPS3_PG_G;
> else
> tlb.tlb_lo0 = mips3_paddr_to_tlbpfn(pa0) | \
> MIPS3_PG_IOPAGE(PMAP_CCA_FOR_PA(pa0));
> if (pa1 == 0)
> tlb.tlb_lo1 = MIPS3_PG_G;
> else
> tlb.tlb_lo1 = mips3_paddr_to_tlbpfn(pa1) | \
> MIPS3_PG_IOPAGE(PMAP_CCA_FOR_PA(pa1));
> mips3_TLBWriteIndexedVPS(MIPS3_TLB_WIRED_UPAGES + nwired, &tlb);
>
> nwired++;
>
> return 0;
>}
>
>/* Allocate new wired entries */
>int
>ews4800mips_map_wired(vaddr_t va, paddr_t pa0, paddr_t pa1, uint32_t pg_size)
>{
> psize_t size;
> int error;
>
> size = MIPS3_PG_SIZE_MASK_TO_SIZE(pg_size);
> va = (va + (size - 1)) & ~(size - 1);
>
> if (va < VM_MIN_WIRED_MAP_ADDRESS ||
> va + size > VM_MAX_WIRED_MAP_ADDRESS) {
>#ifdef DIAGNOSTIC
> printf("ews4800mips_enter_wired: invalid va range.\n");
>#endif
> return EINVAL;
> }
>
> error = extent_alloc_region(wired_map_ex, va, size, EX_NOWAIT);
> if (error) {
>#ifdef DIAGNOSTIC
> printf("ews4800mips_enter_wired: cannot allocate region.\n");
>#endif
> return error;
> }
>
> error = ews4800mips_enter_wired(va, pa0, pa1, pg_size);
>
> return error;
>}
>
>vaddr_t
>ews4800mips_map_wired_region(paddr_t pa, int size, uint32_t pg_mask)
>{
> vaddr_t va, rva;
> vsize_t off;
> psize_t pg_size;
> int error;
>
> pg_size = MIPS3_PG_SIZE_MASK_TO_SIZE(pg_mask);
> off = pa & WIRED_ENTRY_OFFMASK(pg_size);
> pa &= ~(paddr_t)WIRED_ENTRY_OFFMASK(pg_size);
> size += off;
>
> if ((size + WIRED_ENTRY_SIZE(pg_size) - 1) / WIRED_ENTRY_SIZE(pg_size) >
> EWS4800MIPS_TLB_WIRED_ENTRIES - nwired) {
>
>#ifdef DIAGNOSTIC
> printf("ews4800mips_map_wired_region(0x%lx, 0x%lx)"
> ": %d is not enough\n", pa + off, size - off,
> EWS4800MIPS_TLB_WIRED_ENTRIES - nwired);
>#endif
> return 0; /* free wired TLB is not enough */
> }
>
> error = extent_alloc(wired_map_ex, size, WIRED_ENTRY_SIZE(pg_size), 0,
> EX_NOWAIT, &va);
> if (error) {
>#ifdef DIAGNOSTIC
> printf("ews4800mips_map_wired_region: can't allocate region\n");
>#endif
> return 0;
> }
> rva = va;
>
> while (size > 0) {
> ews4800mips_enter_wired(va, pa, pa + pg_size, pg_mask);
> pa += WIRED_ENTRY_SIZE(pg_size);
> va += WIRED_ENTRY_SIZE(pg_size);
> size -= WIRED_ENTRY_SIZE(pg_size);
> }
>
> return rva + off;
>}
>---
>
>---
>/* $NetBSD$ */
>
>/*-
> * Copyright (C) 2005 Izumi Tsutsui.
> * Copyright (C) 2000 Shuichiro URATA. All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions
> * are met:
> * 1. Redistributions of source code must retain the above copyright
> * notice, this list of conditions and the following disclaimer.
> * 2. Redistributions in binary form must reproduce the above copyright
> * notice, this list of conditions and the following disclaimer in the
> * documentation and/or other materials provided with the distribution.
> * 3. The name of the author may not be used to endorse or promote products
> * derived from this software without specific prior written permission.
> *
> * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
>#define WIRED_ENTRY_SIZE(pg_size) ((pg_size) * 2)
>#define WIRED_ENTRY_OFFMASK(pg_size) (WIRED_ENTRY_SIZE(pg_size) - 1)
>
>void ews4800mips_init_wired_map(void);
>int ews4800mips_map_wired(vaddr_t va, paddr_t pa0, paddr_t pa1,
> uint32_t pg_size);
>vaddr_t ews4800mips_map_wired_region(paddr_t pa, int size, uint32_t pg_size);
>---
>
>