Re: Re: PCID support for amd64 - initial patch.

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 31/03/2018 à 22:22, Jaromír Doleček a écrit :
> Hello,
>
> I've fixed the remaining issues WRT SVS, so the current version boots
> and works without apparent problems with and without SVS. I tested
> this inside emulator with PCID/INVPCID support.
>
> Patch:
> http://www.netbsd.org/~jdolecek/pcid_support+svs.diff
>
> This version is probably close to what will eventually get integrated.
> First I want to confirm the functionality on real hardware, and check
> what kind of performance difference does it make for both SVS and
> non-SVS case. I'll share results once I'll have them.

Maybe I read too quickly, but there are two things that seem really wrong
in your patch:

 * Where do you flush the PCID on CPUs that have TLBs of threads they are
   not executing? Say you have lwp1 executing on cpu0. cpu0 switches to
   lwp2. lwp1's TLBs are still stored on cpu0. cpu1 switches to lwp1. lwp1
   does an mmap, munmap, or whatever that modifies the page tables. Here
   you need to send an IPI to cpu0 to ask him to flush its lwp1 ctx. Because
   if you don't, and if cpu0 later switches back to lwp1, the TLB is invalid
   and random memory corruptions occur all over the place. It doesn't seem
   to me that you send the IPI there, do you?

 * The variable array at the end of struct pmap is terrible, and it
   shouldn't be done this way. Just declare it as [MAXCPUS]. Yes, it means
   we allocate a little more memory than necessary in most cases, but
   that's how it is.

Maxime