powerpc64/GENERIC64: possible misuse of tlbsync instruction for moea64_cpu_bootstrap_native? Possible lack of isync instruction?

[Nathan Whitehorn]

This is all fine. These two sets of instructions are somewhat special 
(and one-offs). The tlbsync here is unnecessary, as you note, but 
harmless (it dates to when using tlbie instead of tlbiel). If you think 
it is causing a problem for some reason, you can try swapping it with a 
ptesync only.
-Nathan

On 09/30/14 06:37, Mark Millard wrote:
> 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 “conflicting instruction” 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...
>
> ...
>         /*
>          * Install page table
>          */
>
>         __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 = 0; i < 0xFF000; i += 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;"
>                 : "=r"(msr), "=r"(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=0, 
> 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.
>
>
>
>
>
>
> ===
> Mark Millard
> markmi at dsl-only.net <http://dsl-only.net>
>