Date: Mon, 29 Jul 2013 17:50:48 +0200 From: Zbyszek Bodek <zbb@semihalf.com> To: "freebsd-arm@freebsd.org" <freebsd-arm@freebsd.org> Cc: ray@freebsd.org, Alan Cox <alc@cs.rice.edu> Subject: Re: New pmap-v6.c features and improvements Message-ID: <51F68F58.8060600@semihalf.com> In-Reply-To: <B6F68E56-7452-44D4-9C9D-195221B06380@bsdimp.com> References: <519B6B1C.9060008@semihalf.com> <20130522184232.GA437@jail.io> <519E0D62.5030708@semihalf.com> <51CC4CC1.4020509@semihalf.com> <51D57456.9080504@semihalf.com> <B6F68E56-7452-44D4-9C9D-195221B06380@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------040509090706030205070508 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 04.07.2013 19:34, Warner Losh wrote: > > On Jul 4, 2013, at 7:10 AM, Zbyszek Bodek wrote: > >> On 27.06.2013 16:31, Zbyszek Bodek wrote: >>> On 23.05.2013 14:36, Zbyszek Bodek wrote: >>>> On 22.05.2013 20:42, Ruslan Bukin wrote: >>>>> On Tue, May 21, 2013 at 02:39:56PM +0200, Zbyszek Bodek wrote: >>>>>> Hello Everyone, >>>>>> >>>>>> I would like to introduce another pack of patches for pmap-v6.c and >>>>>> related, that we created as a part of Semihalf work on Superpages >>>>>> support. >>>>>> >>>>>> The patches include some major changes like: >>>>>> >>>>>> - Switch to AP[1:0] access permissions model >>>>>> - Transition of the mapping related flags to PTE (stop using PVF_ flags >>>>>> in pv_entry) >>>>>> - Rework of the pmap_enter_locked() function >>>>>> - pmap code optimizations >>>>>> >>>>>> And some minor clean-ups: >>>>>> >>>>>> - Get rid of the VERBOSE_INIT_ARM option >>>>>> - Clean-ups, style and naming improvements to pmap >>>>>> >>>>>> Please check out the attachment for details. >>>>>> >>>>>> I will be happy to answer your questions and doubts if any. >>>>>> >>>>>> Best regards >>>>>> Zbyszek Bodek >>>>> >>>>> I tested new patches with exynos5 and everything is OK. >>>>> (I mean all works as usual) >>>>> >>>> >>>> Hello. >>>> >>>> I'm happy to announce that code has been integrated to the FreeBSD HEAD. >>>> Great thanks your help! >>>> >>> >>> Hello Everyone, >>> >>> We have two micro patches for pmap-v6.c containing fix for 'modified' >>> bit emulation and removal of the redundant PGA_WRITEABLE clearing. >>> >>> Please check out the attachment. >>> >>> These two are minimal changes and we would like to commit them soon, so >>> we would be grateful if you could test them on your ARMv6/v7 platforms >>> and give us your remarks. >>> >> >> Hello, >> >> Because there were no objections, we've integrated the patches: >> >> http://svnweb.freebsd.org/base?view=revision&revision=252694 >> http://svnweb.freebsd.org/base?view=revision&revision=252695 > Hello Everyone, I'm sending another set of fixes for pmap-v6.c Please see attachment. 0012 - Removal of the costly, frequent sweeping through the pv_entries whenever pmap_nuke_pv(), pmap_modify_pv(), etc. are called. This also makes order with clearing PGA_WRITEABLE aflag as well as with the pmap_statistics related to the wired_count. 0013 - Fix for pamp_set_prot() where not all of the protection bits were cleared before the actual configuration. L2_XN is not icluded to the L2_S_PROT mask to maintain consistency between PROT_MASKS for L2 and L1 descriptors - contain only kernel/user and read/write permissions bits. 0014 - Fix for pmap_change_wiring() where "wired" indicator of type boolean was being used as a "wired flag" passed to pmap_modify_pv() which was obviously not a valid PVF_WIRED flag. Please test those patches on your ARM-based machines and send your feedback. We will also appreciate your reviews and remarks. Thank you in advance for your help. Best regards Zbyszek Bodek --------------040509090706030205070508 Content-Type: text/x-patch; name="0012-Clarify-pv_entry-removal-in-respect-to-pmap-statisti.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0012-Clarify-pv_entry-removal-in-respect-to-pmap-statisti.pa"; filename*1="tch" >From 39cd0d193105e3417f7251c6faa127042808af2f Mon Sep 17 00:00:00 2001 From: Zbigniew Bodek <zbb@semihalf.com> Date: Wed, 24 Jul 2013 20:19:38 +0200 Subject: [PATCH 1/3] Clarify pv_entry removal in respect to pmap statistics and PGA_ flags for ARMv6/v7 platforms - PGA_WRITEABLE indicates that there *might be* a writable mapping for the particular page so to avoid frequent sweeping of the pv_entries whenever pmap_nuke_pv(), pmap_modify_pv(), etc. is called it is sufficient to clear that flag if there are no managed mappings for that page anymore (notice that only pmap_enter is authorized to set this flag). - Avoid redundant checking for PVF_WIRED flag when this flag cannot be set anyway. - Clear PGA_WRITEABLE only once for each vm_page instead of multiple, redundant clearing it in loop when there are no writeable mappings to that page anymore. Submitted by: Zbigniew Bodek <zbb@semihalf.com> Sponsored by: The FreeBSD Foundation, Semihalf --- sys/arm/arm/pmap-v6.c | 54 +++++++++++++++++---------------------------------- 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/sys/arm/arm/pmap-v6.c b/sys/arm/arm/pmap-v6.c index 73b899c..15eb5e1 100644 --- a/sys/arm/arm/pmap-v6.c +++ b/sys/arm/arm/pmap-v6.c @@ -1072,39 +1072,22 @@ pmap_set_prot(pt_entry_t *ptep, vm_prot_t prot, uint8_t user) * => caller should NOT adjust pmap's wire_count * => we return the removed pve */ - -static void -pmap_nuke_pv(struct vm_page *m, pmap_t pmap, struct pv_entry *pve) -{ - - rw_assert(&pvh_global_lock, RA_WLOCKED); - PMAP_ASSERT_LOCKED(pmap); - - TAILQ_REMOVE(&m->md.pv_list, pve, pv_list); - - if (pve->pv_flags & PVF_WIRED) - --pmap->pm_stats.wired_count; - - if (pve->pv_flags & PVF_WRITE) { - TAILQ_FOREACH(pve, &m->md.pv_list, pv_list) - if (pve->pv_flags & PVF_WRITE) - break; - if (!pve) { - vm_page_aflag_clear(m, PGA_WRITEABLE); - } - } -} - static struct pv_entry * pmap_remove_pv(struct vm_page *m, pmap_t pmap, vm_offset_t va) { struct pv_entry *pve; rw_assert(&pvh_global_lock, RA_WLOCKED); + PMAP_ASSERT_LOCKED(pmap); pve = pmap_find_pv(m, pmap, va); /* find corresponding pve */ - if (pve != NULL) - pmap_nuke_pv(m, pmap, pve); + if (pve != NULL) { + TAILQ_REMOVE(&m->md.pv_list, pve, pv_list); + if (pve->pv_flags & PVF_WIRED) + --pmap->pm_stats.wired_count; + } + if (TAILQ_EMPTY(&m->md.pv_list)) + vm_page_aflag_clear(m, PGA_WRITEABLE); return(pve); /* return removed pve */ } @@ -1143,14 +1126,6 @@ pmap_modify_pv(struct vm_page *m, pmap_t pmap, vm_offset_t va, else --pmap->pm_stats.wired_count; } - if ((oflags & PVF_WRITE) && !(flags & PVF_WRITE)) { - TAILQ_FOREACH(npv, &m->md.pv_list, pv_list) { - if (npv->pv_flags & PVF_WRITE) - break; - } - if (!npv) - vm_page_aflag_clear(m, PGA_WRITEABLE); - } return (oflags); } @@ -2063,7 +2038,9 @@ pmap_remove_pages(pmap_t pmap) pv_entry_count--; pmap->pm_stats.resident_count--; pc->pc_map[field] |= bitmask; - pmap_nuke_pv(m, pmap, pv); + TAILQ_REMOVE(&m->md.pv_list, pv, pv_list); + if (TAILQ_EMPTY(&m->md.pv_list)) + vm_page_aflag_clear(m, PGA_WRITEABLE); pmap_free_l2_bucket(pmap, l2b, 1); } } @@ -2459,7 +2436,9 @@ pmap_remove_all(vm_page_t m) PTE_SYNC(ptep); pmap_free_l2_bucket(pmap, l2b, 1); pmap->pm_stats.resident_count--; - pmap_nuke_pv(m, pmap, pv); + TAILQ_REMOVE(&m->md.pv_list, pv, pv_list); + if (pv->pv_flags & PVF_WIRED) + pmap->pm_stats.wired_count--; pmap_free_pv_entry(pmap, pv); PMAP_UNLOCK(pmap); } @@ -2470,6 +2449,7 @@ pmap_remove_all(vm_page_t m) else cpu_tlb_flushD(); } + vm_page_aflag_clear(m, PGA_WRITEABLE); rw_wunlock(&pvh_global_lock); } @@ -3339,7 +3319,9 @@ pmap_pv_reclaim(pmap_t locked_pmap) "va %x pte %x", va, *ptep)); *ptep = 0; PTE_SYNC(ptep); - pmap_nuke_pv(m, pmap, pv); + TAILQ_REMOVE(&m->md.pv_list, pv, pv_list); + if (TAILQ_EMPTY(&m->md.pv_list)) + vm_page_aflag_clear(m, PGA_WRITEABLE); pc->pc_map[field] |= 1UL << bit; freed++; } -- 1.8.2 --------------040509090706030205070508 Content-Type: text/x-patch; name="0013-Clear-all-of-the-protection-bits-in-L2-PTE-before-th.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0013-Clear-all-of-the-protection-bits-in-L2-PTE-before-th.pa"; filename*1="tch" >From 0f1e316cd5ce2fe60d06cbd4e3a9d3f96aa79561 Mon Sep 17 00:00:00 2001 From: Zbigniew Bodek <zbb@semihalf.com> Date: Mon, 29 Jul 2013 00:06:10 +0200 Subject: [PATCH 2/3] Clear all of the protection bits in L2 PTE before their configuration. Revise L2_S_PROT_MASK to include all of the protection bits. Notice that clearing those bits does not always take away the corresponding permissions (in example the permission is granted when the bit is cleared). The bits are cleared but are to be set or left cleared accordingly in pmap_set_prot(), pmap_enter_locked(), etc. Clear L2_XN along with L2_S_PROT_MASK in pmap_set_prot() so that all permissions related bits are cleared before the actual configuration. Submitted by: Zbigniew Bodek <zbb@semihalf.com> Sponsored by: The FreeBSD Foundation, Semihalf --- sys/arm/arm/pmap-v6.c | 2 +- sys/arm/include/pmap.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sys/arm/arm/pmap-v6.c b/sys/arm/arm/pmap-v6.c index 15eb5e1..cbcecb8 100644 --- a/sys/arm/arm/pmap-v6.c +++ b/sys/arm/arm/pmap-v6.c @@ -1046,7 +1046,7 @@ static void pmap_set_prot(pt_entry_t *ptep, vm_prot_t prot, uint8_t user) { - *ptep &= ~L2_S_PROT_MASK; + *ptep &= ~(L2_S_PROT_MASK | L2_XN); if (!(prot & VM_PROT_EXECUTE)) *ptep |= L2_XN; diff --git a/sys/arm/include/pmap.h b/sys/arm/include/pmap.h index ec40682..b416169 100644 --- a/sys/arm/include/pmap.h +++ b/sys/arm/include/pmap.h @@ -390,7 +390,7 @@ extern int pmap_needs_pte_sync; #define L2_S_PROT_U (L2_AP0(2)) /* user read */ #define L2_S_REF (L2_AP0(1)) /* reference flag */ -#define L2_S_PROT_MASK (L2_S_PROT_U|L2_S_PROT_R) +#define L2_S_PROT_MASK (L2_S_PROT_U|L2_S_PROT_R|L2_APX) #define L2_S_EXECUTABLE(pte) (!(pte & L2_XN)) #define L2_S_WRITABLE(pte) (!(pte & L2_APX)) #define L2_S_REFERENCED(pte) (!!(pte & L2_S_REF)) -- 1.8.2 --------------040509090706030205070508 Content-Type: text/x-patch; name="0014-Fix-ARMv6-v7-mapping-s-wired-status-configuration-in.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0014-Fix-ARMv6-v7-mapping-s-wired-status-configuration-in.pa"; filename*1="tch" >From f11e954a428e0909192e5754591ab0a3cc494630 Mon Sep 17 00:00:00 2001 From: Zbigniew Bodek <zbb@semihalf.com> Date: Mon, 29 Jul 2013 13:23:57 +0200 Subject: [PATCH 3/3] Fix ARMv6/v7 mapping's wired status configuration in pmap_change_wiring() Last input argument in pmap_modify_pv() should be a mask of flags to be set. In pmap_change_wiring() however, the wired status has been used for that purposes which is not representing any of the valid flags and is of type boolean. This commit fixes that issue so that wired flag is passed to pmap_modify_pv() properly. Submitted by: Zbigniew Bodek <zbb@semihalf.com> Sponsored by: The FreeBSD Foundation, Semihalf --- sys/arm/arm/pmap-v6.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sys/arm/arm/pmap-v6.c b/sys/arm/arm/pmap-v6.c index cbcecb8..3142f56 100644 --- a/sys/arm/arm/pmap-v6.c +++ b/sys/arm/arm/pmap-v6.c @@ -2933,7 +2933,8 @@ pmap_change_wiring(pmap_t pmap, vm_offset_t va, boolean_t wired) pte = *ptep; m = PHYS_TO_VM_PAGE(l2pte_pa(pte)); if (m != NULL) - pmap_modify_pv(m, pmap, va, PVF_WIRED, wired); + pmap_modify_pv(m, pmap, va, PVF_WIRED, + wired == TRUE ? PVF_WIRED : 0); rw_wunlock(&pvh_global_lock); PMAP_UNLOCK(pmap); } -- 1.8.2 --------------040509090706030205070508--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51F68F58.8060600>