Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 30 Sep 2014 06:37:15 -0700
From:      Mark Millard <markmi@dsl-only.net>
To:        Nathan Whitehorn <nwhitehorn@freebsd.org>, FreeBSD PowerPC ML <freebsd-ppc@freebsd.org>
Cc:        Justin Hibbits <chmeeedalf@gmail.com>
Subject:   powerpc64/GENERIC64: possible misuse of tlbsync instruction for moea64_cpu_bootstrap_native? Possible lack of isync instruction?
Message-ID:  <901E3FCC-1387-46B0-98D4-78EF286D6E6C@dsl-only.net>

next in thread | raw e-mail | index | archive | help
PowerISA_V2.07_PUBLIC.pdf page labeled 935, section 5.10.1 Page table =
Updates says of tlbsync...

Software must execute tlbie and tlbsync instructions only as part of the =
following sequence, and must ensure that no other thread will execute a =
=93conflicting instruction=94 while the instructions in the sequence are =
executing on the given thread. In addition to achieving the required =
system synchronization, the sequence will cause transactions that =
include accesses to the affected page(s) to fail.

tlbie instruction(s) specifying the same LPID oper- and value
eieio
tlbsync

ptesync

There is also the prior page (labeled 934) which says (and older =
documentation seems to agree)...

tlbsync should not be used to synchronize the completion of tlbiel.

[Other places in the document also report that. Some older documents are =
explicit about ptesync use: "To synchronize the completion of this =
processor local form of tlbie, only a ptesync is required (tlbsync =
should not be used)." ]

But /usr/src/sys/powerpc/aim/moea64_native.c has =
moea64_cpu_bootstrap_native(...) doing...=20

...
        /*
         * Install page table
         */
=20
        __asm __volatile ("ptesync; mtsdr1 %0; isync"
            :: "r"((uintptr_t)moea64_pteg_table
                     | (uintptr_t)(flsl(moea64_pteg_mask >> 11))));
        tlbia();
}

which in turn does...

static void
tlbia(void)
{
        vm_offset_t i;
        #ifndef __powerpc64__
        register_t msr, scratch;
        #endif

        TLBSYNC();

        for (i =3D 0; i < 0xFF000; i +=3D 0x00001000) {
                #ifdef __powerpc64__
                __asm __volatile("tlbiel %0" :: "r"(i));
                #else
                __asm __volatile("\
                    mfmsr %0; \
                    mr %1, %0; \
                    insrdi %1,%3,1,0; \
                    mtmsrd %1; \
                    isync; \
                    \
                    tlbiel %2; \
                    \
                    mtmsrd %0; \
                    isync;"
                : "=3Dr"(msr), "=3Dr"(scratch) : "r"(i), "r"(1));
                #endif
        }

        EIEIO();
        TLBSYNC();
}

which involves...

#define TLBSYNC()       __asm __volatile("tlbsync; ptesync");

The first TLBSYNC() above uses tlbsync without being part of a tlbie; =
eieio; tlbsync; ptesync sequence, violating the first point that I =
quoted.

The second also has that property but there was also a tlbiel used =
first, apparently violating the second point that I quoted.


tlbiel is documented in boot III-S as requiring a context-synchronizing =
instruction first and a ptesync after for data access. For instruction =
access: nothing before but context-synchronizing after. No tlbsync is =
listed as required.

For the __powerpc64__ above the tlbiel's are not followed by a =
context-sychronizing instruction (sc, isync, rfid, mtmsr[d] with L=3D0, =
etc.) and so would need to not effectively be involved in later =
instruction access. (The ptesync is listed as execution synchronizing, =
not context synchronizing.)



Another quote about tlbiel (page labeled 933):

The primary use of this instruction by operating system software is to =
invalidate TLB entries that were created by the hypervisor using an =
implemen- tation-specific hypervisor-managed TLB facility, if such a =
facility is provided.






=3D=3D=3D
Mark Millard
markmi at dsl-only.net




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?901E3FCC-1387-46B0-98D4-78EF286D6E6C>