Handle pmap_enter() over an existing 4/2M page on i386.

[kib]

Summary

This change apparently sit in my local tree for long time.

I do not see why it is impossible for e.g. two threads to fault on the same address simultaneously, then one thread creates the mapping in pmap_enter() and promotes it, and only then the second thread enters pmap_enter(), either due to locking or locking and scheduling. In this case, amd64 pmap demotes large page first, while i386 panics for some contrast.

Handle the i386 pmap_enter() same as amd64, demote.

Test Plan

I believe it was done after some actual Peter report of 'panic("pmap_enter: attempted pmap_enter on 4MB page")', but the situation is very hard to reproduce.

[alc]

Unless the mapping is a kernel virtual address, the demotion is already being performed at the start of pmap_allocpte().

[kib]

In D8323#173239, @alc wrote:

Unless the mapping is a kernel virtual address, the demotion is already being performed at the start of pmap_allocpte().

Yes. It is probably practically impossible to have a promotion on the kernel (sub)map entry handled by pmap_enter() on i386.

But still, I would like to either add the demotion call for KVA just for completeness, or a comment noting what you said + note to the panic that it is for kernel addresses. What do you prefer ?

[alc]

In D8323#173301, @kib wrote:

In D8323#173239, @alc wrote:

Unless the mapping is a kernel virtual address, the demotion is already being performed at the start of pmap_allocpte().

Yes. It is probably practically impossible to have a promotion on the kernel (sub)map entry handled by pmap_enter() on i386.

But still, I would like to either add the demotion call for KVA just for completeness, or a comment noting what you said + note to the panic that it is for kernel addresses. What do you prefer ?

Let's do the former. I don't see a downside to making it work for KVA.

sys/i386/i386/pmap.c

3472 ↗

(On Diff #21614)

If pmap_demote_pde() can't allocate a page table page (and returns FALSE), then we need to execute the "else if". Take a look at how I wrote this on amd64 with the demotion call included in the test expression of the "if". You need to do the same here.

[kib]

For KVA, pde must exist and be valid, and demotion failure is impossible, due to pmap_growkernel() preallocating the page table page, am I wrong ? If not, I propose to leave the UVA case as is, and just call pmap_demote_pde() for KVA.

[kib]

Only add code for KVA demotion.

[alc]

In D8323#173555, @kib wrote:

For KVA, pde must exist and be valid, and demotion failure is impossible, due to pmap_growkernel() preallocating the page table page, am I wrong ? If not, I propose to leave the UVA case as is, and just call pmap_demote_pde() for KVA.

You are correct.

sys/i386/i386/pmap.c

3488 ↗

(On Diff #21666)

"va is for KVA, so pmap_demote_pde() will never fail to install a page table page.

[alc]

sys/i386/i386/pmap.c

3500 ↗

(On Diff #21666)

As an aside to this change, the above comment makes no sense. The page table page allocation has already occurred.

[kib]

Update comments.

[alc]

sys/i386/i386/pmap.c

3501 ↗

(On Diff #21673)

You use "should" in the next sentence, and these two sentences are related. I would use "should" here as well.

3502 ↗

(On Diff #21673)

"... should have either ..."

3503 ↗

(On Diff #21673)

You don't need the "," here.

[alc]

sys/i386/i386/pmap.c

3503 ↗

(On Diff #21673)

Also, "... demoted the existing ..."

[kib]

Grammar fixes.