Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 9 May 2015 22:25:39 +0200
From:      Oliver Pinter <oliver.pinter@hardenedbsd.org>
To:        Konstantin Belousov <kib@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r282684 - in head/sys: amd64/amd64 amd64/include x86/include x86/xen
Message-ID:  <CAPQ4ffv_mYWRcYV7k1uGXySotjojk_ZKsD_v7bJWRWUo2sC1Ew@mail.gmail.com>
In-Reply-To: <201505091911.t49JB2gh067512@svn.freebsd.org>
References:  <201505091911.t49JB2gh067512@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Konstantin!

On 5/9/15, Konstantin Belousov <kib@freebsd.org> wrote:
> Author: kib
> Date: Sat May  9 19:11:01 2015
> New Revision: 282684
> URL: https://svnweb.freebsd.org/changeset/base/282684
>
> Log:
>   Rewrite amd64 PCID implementation to follow an algorithm described in
>   the Vahalia' "Unix Internals" section 15.12 "Other TLB Consistency
>   Algorithms".  The same algorithm is already utilized by the MIPS pmap
>   to handle ASIDs.
>
>   The PCID for the address space is now allocated per-cpu during context
>   switch to the thread using pmap, when no PCID on the cpu was ever
>   allocated, or the current PCID is invalidated.  If the PCID is reused,
>   bit 63 of %cr3 can be set to avoid TLB flush.
>
>   Each cpu has PCID' algorithm generation count, which is saved in the
>   pmap pcpu block when pcpu PCID is allocated.  On invalidation, the
>   pmap generation count is zeroed, which signals the context switch code
>   that already allocated PCID is no longer valid.  The implication is
>   the TLB shootdown for the given cpu/address space, due to the
>   allocation of new PCID.
>
>   The pm_save mask is no longer has to be tracked, which (significantly)
>   reduces the targets of the TLB shootdown IPIs.  Previously, pm_save
>   was reset only on pmap_invalidate_all(), which made it accumulate the
>   cpuids of all processors on which the thread was scheduled between
>   full TLB shootdowns.
>
>   Besides reducing the amount of TLB shootdowns and removing atomics to
>   update pm_saves in the context switch code, the algorithm is much
>   simpler than the maintanence of pm_save and selection of the right
>   address space in the shootdown IPI handler.
>
>   Reviewed by:	alc
>   Tested by:	pho
>   Sponsored by:	The FreeBSD Foundation
>   MFC after:	3 weeks
>
> Modified:
>   head/sys/amd64/amd64/apic_vector.S
>   head/sys/amd64/amd64/cpu_switch.S
>   head/sys/amd64/amd64/genassym.c
>   head/sys/amd64/amd64/machdep.c
>   head/sys/amd64/amd64/mp_machdep.c
>   head/sys/amd64/amd64/pmap.c
>   head/sys/amd64/amd64/vm_machdep.c
>   head/sys/amd64/include/cpufunc.h
>   head/sys/amd64/include/pcpu.h
>   head/sys/amd64/include/pmap.h
>   head/sys/amd64/include/smp.h
>   head/sys/x86/include/specialreg.h
>   head/sys/x86/xen/xen_apic.c

...

The KASSERT should be ```KASSERT(pmap->pm_pcids[cpuid].pm_pcid !=
PMAP_PCID_KERN || pmap == kernel_pmap,``` instead of the current
```KASSERT(pmap != PMAP_PCID_KERN || pmap == kernel_pmap,```.

You compared the pmap against the kernel pmap's PCID, instead of the
pmap's pcid.
https://github.com/freebsd/freebsd/commit/3fb738761ee4e1438402fd537fc893b44ae9312b#diff-9e67c23e66565ca69fc2e6a06631f43eR6605



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4ffv_mYWRcYV7k1uGXySotjojk_ZKsD_v7bJWRWUo2sC1Ew>