moea64_cpu_bootstrap_native slbmte (and such) use: missing context synchronizations (isync's)?

[Mark Millard]

[Looks like slb_insert_kernel also does not locally
have context synchronization around slbmte and I've
a question on slbie after slbmte as well.]

On 2019-Apr-30, at 17:25, Mark Millard <marklmi at yahoo.com> wrote:

> First some context that I base this note on . . .
> 
> For Data Access Synchronization:
> 
> slbie, slbia, slbmte all list a context-synchronizing instruction
> as "Required Prior" and "Required After".
> 
> For Instruction Access Synchronization:
> 
> slbie, slbia, slbmte all list a context-synchronizing instruction
> as "Required After". (There is also a note about not causing an
> implicit branch in real space, up to the context synchronization
> instruction.)
> 
> Both contexts list the note that says:
> 
> QUOTE
> If an slbmte instruction alters the mapping, or associated attributes, of a currently mapped ESID, the slb- mte must be preceded by an slbie (or slbia) instruction that invalidates the existing translation. This applies even if the corresponding entry is no longer in the SLB (the translation may still be in implementa- tion-specific address translation lookaside information). No software synchronization is needed between the slbie and the slbmte, regardless of whether the index of the SLB entry (if any) containing the current translation is the same as the SLB index specified by the slbmte.
> 
> No slbie (or slbia) is needed if the slbmte instruction replaces a valid SLB entry with a mapping of a dif- ferent ESID (e.g., to satisfy an SLB miss). However, the slbie is needed later if and when the translation that was contained in the replaced SLB entry is to be invalidated.
> END QUOTE
> 
> In:
> 
> static void
> moea64_cpu_bootstrap_native(mmu_t mmup, int ap)
> {
> . . .
>        /*
>         * Install kernel SLB entries
>         */
> 
>        #ifdef __powerpc64__
>                __asm __volatile ("slbia");
>                __asm __volatile ("slbmfee %0,%1; slbie %0;" : "=r"(seg0) :
>                    "r"(0));
> 
>                for (i = 0; i < n_slbs; i++) {
>                        if (!(slb[i].slbe & SLBE_VALID))
>                                continue;
> 
>                        __asm __volatile ("slbmte %0, %1" ::
>                            "r"(slb[i].slbv), "r"(slb[i].slbe));
>                }
>        #else
> 
> I do not see isync's or such used for this unless
> the later else-case in:
> 
>        if (cpu_features2 & PPC_FEATURE2_ARCH_3_00)
>                mtspr(SPR_PTCR,
>                    ((uintptr_t)moea64_part_table & ~DMAP_BASE_ADDRESS) |
>                     flsl((PART_SIZE >> 12) - 1));
>        else
>                __asm __volatile ("ptesync; mtsdr1 %0; isync"
>                    :: "r"(((uintptr_t)moea64_pteg_table & ~DMAP_BASE_ADDRESS)
>                             | (uintptr_t)(flsl(moea64_pteg_mask >> 11))));
>        tlbia();
> 
> is supposed to always be sufficient (for at least after
> the slbmte). TLBSYNC(), EIEIO(), and such from tlbia()
> do not seem to do isync, unless I missed something. It
> looks like Power9 (PPC_FEATURE2_ARCH_3_00) would not get
> an isync at all from the range of code I reference.
> 


In:

void
slb_insert_kernel(uint64_t slbe, uint64_t slbv)
{
        struct slb *slbcache;
        int i;

        /* We don't want to be preempted while modifying the kernel map */
        critical_enter();

. . .

fillkernslb:
        KASSERT(i != USER_SLB_SLOT,
            ("Filling user SLB slot with a kernel mapping"));
        slbcache[i].slbv = slbv;
        slbcache[i].slbe = slbe | (uint64_t)i;

        /* If it is for this CPU, put it in the SLB right away */
        if (pmap_bootstrapped) {
                /* slbie not required */
                __asm __volatile ("slbmte %0, %1" ::
                    "r"(slbcache[i].slbv), "r"(slbcache[i].slbe));
        }

        critical_exit();
}

there does not seem to be context synchronizing instructions
before or after the slbmte.

Also, while a slbie before may not be needed, I'm unclear on the
status of a potential slbie needed later:

QUOTE
the slbie is needed later if and when the translation that was 
contained in the replaced SLB entry is to be invalidated
END QUOTE

This slbie-after-slbmte may apply to moea64_cpu_bootstrap_native's
use of slbmte as well.

moea64_deactivate has just one side of slbie with a synchronizing
instruction:

void
moea64_deactivate(mmu_t mmu, struct thread *td)
{
        pmap_t  pm;

        __asm __volatile("isync; slbie %0" :: "r"(USER_ADDR));
. . .

restore_usersrs and restore_kernsrs have a similar
status relative to slbia, but with an interrupt
context it may be that, for example, the rti covers
following context-synchronization sufficiently,
in other words, the internal-interrupt-code possibly
having no need for such(?).

===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)