Use PCID to optimize PTI.

[kib]

Summary

Use PCID to avoid complete TLB shootdown when switching between user and kernel mode with PTI enabled.

I use the model close to what I read about KAISER, user-mode PCID has 1:1 correspondence to the kernel-mode PCID, by setting bit 11 in PCID. Full kernel-mode TLB shootdown is performed on context switches, since KVA TLB invalidation only works in the current pmap. On the other hand, on kernel/user switches, CR3_PCID_SAVE bit is set and we do not clear TLB.

I can imagine alternative use of PCID, where there is only one PCID allocated for the kernel pmap. Then, there is no need to shootdown kernel TLB entries on context switch. But copyout(3) would need to either use method similar to proc_rwmem() to access the userspace data, or (in reverse) provide a temporal mapping for the kernel buffer into user mode PCID and use trampoline for copy.

Below is the comparison of the PTI overhead on a haswell machine, which has INVPCID instruction. Patch is not yet tested, I only booted multiuser on haswell (INVPCID) and sandy (PCID but no INVPCID).

./syscall_timing_64 getppid

PCID pti
Clock resolution: 0.000000001
test loop time iterations periteration
getppid 0 1.013932533 2119160 0.000000478
getppid 1 1.018957555 2129562 0.000000478
getppid 2 1.000971159 2091990 0.000000478
getppid 3 1.000953178 2091982 0.000000478
getppid 4 1.020247113 2131976 0.000000478
getppid 5 1.037923614 2169751 0.000000478
getppid 6 1.047713414 2189130 0.000000478
getppid 7 1.038963785 2171784 0.000000478
getppid 8 1.039963943 2173825 0.000000478
getppid 9 1.035961526 2165531 0.000000478

PCID !pti
Clock resolution: 0.000000001
test loop time iterations periteration
getppid 0 1.012187691 2985156 0.000000339
getppid 1 1.023738555 3018773 0.000000339
getppid 2 1.042912187 3075443 0.000000339
getppid 3 1.043958246 3078485 0.000000339
getppid 4 1.043985730 3078480 0.000000339
getppid 5 1.042927727 3075419 0.000000339
getppid 6 1.043948467 3078553 0.000000339
getppid 7 1.035955084 3054875 0.000000339
getppid 8 1.000223259 2949554 0.000000339
getppid 9 1.007950158 2972239 0.000000339

!PCID pti
Clock resolution: 0.000000001
test loop time iterations periteration
getppid 0 1.038952102 1719328 0.000000604
getppid 1 1.024812746 1695689 0.000000604
getppid 2 1.008090182 1667812 0.000000604
getppid 3 1.042990431 1725911 0.000000604
getppid 4 1.043929676 1726714 0.000000604
getppid 5 1.042965631 1725831 0.000000604
getppid 6 1.043961847 1727683 0.000000604
getppid 7 1.042965348 1726560 0.000000604
getppid 8 1.043959939 1727769 0.000000604
getppid 9 1.043970836 1727738 0.000000604

Test Plan

Peter, could you, please, test the patch ?

I did several simple smoke-testing actions, in particular, compiled the openjdk port on debug and production kernel. It is interesting to test this both on machine which reports INVPCID CPU feature, and on machine with PCID but without INVPCID, amd64 only.

Thank you in advance.

[emaste]

This passes smoke test on my Sandy Bridge laptop (X220).

[kib]

Fix i386 compilation.

[kib]

Merge with head.

[markj]

sys/amd64/amd64/mp_machdep.c

468

IMHO it would be clearer to write

if (smp_tlb_pmap == kernel_pmap) {
    d.pcid = 0;
    invpcid(&d, INVPCID_CTXGLOB);
} else {
    d.pcid = smp_tlb_pmap->pm_pcids[PCPU_GET(cpuid)].pm_pcid;
    invpcid(&d, INVPCID_CTX);
    d.pcid |= PMAP_PCID_USER_PT;
    invpcid(&d, INVPCID_CTX);
}

505

Perhaps cache the cpuid here and in the other handlers, rather than fetching from pcpu area multiple times.

563

Seems we are missing the user PCID bit here?

600

Extra newline.

sys/amd64/amd64/pmap.c

1599

Why does this need to be a critical section?

sys/amd64/amd64/support.S

807

Can't these routines be implemented in C?

[kib]

sys/amd64/amd64/pmap.c

1599

pm_pcid are recalculated on the context switch. If we are removed from CPU and then rescheduled (pin still allows that) we would end up using incorrect pcid for the operation.

sys/amd64/amd64/support.S

807

This is quite risky. We are temporary swapping address spaces there, in particular, we enter the AS where the C stack is not mapped. I absolutely need to ensure that the routines do not access anything outside the explicitly allowed addresses while on user page table.

[kib]

Shoot down user page table TLB on context switches. It is done automatically only for the kernel twin.
FIx bug pointed out by Mark where wrong PCID was used for user table shoot down on non-INVPCID machine.
Minor editings.

[kib]

When reloading CR3 for shoot down, do not set the save bit.

[markj]

sys/amd64/amd64/mp_machdep.c

509

Need the user bit here too.

sys/amd64/amd64/support.S

807

Could you please add prototypes like in the functions above and in cpu_switch.S?

[kib]

Add comments to support.S pmap_pti functions.
One more missed USER_PT bit.

[kib]

Do not call pmap_pti_pcid_invalidate() for pmaps which do not have usermode page tables.

[markj]

sys/amd64/amd64/support.S

807

Hm, so isn't this unsafe wrt NMIs?

[markj]

sys/amd64/amd64/mp_machdep.c

632

Missing user bit. :)

635

Extra newline.

[markj]

sys/amd64/amd64/support.S

807

Never mind, sorry. I forgot that the NMI handler reloads from KCR3.

[kib]

sys/amd64/amd64/support.S

807

Yes, there are many more places where kernel is executing with UCR3, and NMI/MCE handlers already handle that.

[kib]

Mark another finding of missed USER_PT.
Update pcpu kcr3 and ucr3 only after shootdown.
Add some comments.

[alc]

sys/amd64/amd64/pmap.c

1686

Stray blank line.

1752

Stray blank line.

[kib]

For PCID+non-INVPCID, only reload kcr3 from pmap in invalidation IPI handler when the pmap is current. Otherwise we end up with invalid translation loaded for usermode part.
Check for valid ucr3 before committing to do the userspace part invalidation.

[kib]

Diff 38279 survived full make -j 32 tinderbox run on ivy bridge machine (PCID but no INVPCID).

[markj]

I can't see any problems in this version.

sys/amd64/amd64/apic_vector.S

188

Is there a reason for not having a separate PTI entry point, as you do for invlpg and invlrng?

sys/amd64/amd64/mp_machdep.c

499

"achieve it" or "effect it" sound more natural than "reach it" to me.

631

Logically it feels like pmap_pti_pcid_invl{pg,rng}() should be setting CR3_PCID_SAVE rather than having it be set in the caller. It would also make the non-INVPCID handlers more consistent.

sys/amd64/amd64/pmap.c

7344

There's a stray space here.

[alc]

sys/amd64/amd64/mp_machdep.c

499

"... to clear kernel mappings from the TLB in the ..."

Also, I'm confused. The "if" expression is "!=". Shouldn't it be "==".

[kib]

sys/amd64/amd64/mp_machdep.c

631

I did not the bit set in assembly to keep the assembler source as small as possible. If you consider this important, I will change the asm.

Note that for the support.S' reader, having CR3_PCID_SAVE set in the caller is more natural (IMHO).

[kib]

Regularize IPI handlers names.
Fix if() condition.
Update comments.

[alc]

sys/amd64/amd64/mp_machdep.c

143

I don't think that you need a continuation line here.

500

I fear that no one (besides us) will understand this comment.

Isn't the issue here that there are now multiple kernel page tables/address spaces, and we want the kernel mapping(s) flushed from all of them?

[kib]

sys/amd64/amd64/mp_machdep.c

500

No, the kernel mappings as well as user mappings are always flushed on context switch, even with PCID enabled. I have to do this because kernel mode PCID are per-pmap. So only the currently active TLB partition is to be flushed.

What I am trying to say there is that I need to know PCID of the pmap where to flush the kernel mappings, this is most likely the PCID for the current pmap. But since the function was asked to do this for the kernel pmap, to not reach out and find the corresponding PCID (can it be non-current after all ?), I use the global flush.

[markj]

sys/amd64/amd64/mp_machdep.c

631

I don't think it's important.